Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify typeof .userData property #656

Closed
hanvit-eazel opened this issue Oct 30, 2023 · 2 comments · Fixed by #694
Closed

Unify typeof .userData property #656

hanvit-eazel opened this issue Oct 30, 2023 · 2 comments · Fixed by #694
Labels
enhancement New feature or request

Comments

@hanvit-eazel
Copy link

Description

Issue submitted in three.js first
link
And also submitted in definitely typed at second
link

Here are the classes that currently use the userData property in three.js.
(captured from @types/three r155.1)

hasUserData
  • Object3D
  • BufferGeometry
  • Material
  • Texture
  • (UserDataNode, GLTF)

But as you can see, Object3D, BufferGeometry and Material, Texture have different userData types.

This situation caused me the following type issue.

error
type UserDataWrappedObject3D = Object3D & {
    userData: { myCustomValue: number }
}

type UserDataWrappedMaterial = Material & {
    userData: { myCustomValue: number }
}

let a: UserDataWrappedObject3D 
let b: UserDataWrappedMaterial 

a.userData.myCustomValue
b.userData.myCustomValue // error from userData inferred as any

Solution

If we want to allow wrapping of userData in the above manner, I suggest replacing the userData type of Materials and Textures with Object3D or BufferGeometry's userData type.

// in Material class
...
userData: { [key: string]: any };
...

Alternatives

Or we can apply Record type for userData as it commented in code.

Additional context

No response

@hanvit-eazel hanvit-eazel added the enhancement New feature or request label Oct 30, 2023
@hanvit-eazel
Copy link
Author

hanvit-eazel commented Oct 30, 2023

Additionally, overwrittening .data property of the Source class is not working correctly too.

This is solved by replacing any with unknown, I would suggest using unknown as normal lint rules.

let a: DataWrappedSource

type DataWrappedSource = Source & {
    data: HTMLImageElement // eg
}

a.data // data inferred as any
problem

@Methuselah96
Copy link
Contributor

Methuselah96 commented Nov 27, 2023

Additionally, overwrittening .data property of the Source class is not working correctly too.

This is a separate issue, if you could create a new issue for this so it's progress can be tracked separately, that would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants