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

Convert Boost to tsx #128

Merged
merged 6 commits into from Jun 13, 2021
Merged

Conversation

bjornstar
Copy link
Member

There are 4 typescript errors that I don't know how to deal with:

src/effects/Boost.tsx:27:9 - error TS2531: Object is possibly 'null'.

27         ref.current.setMatrixAt(i + j, o.matrixWorld)
           ~~~~~~~~~~~

src/effects/Boost.tsx:28:9 - error TS2531: Object is possibly 'null'.

28         ref.current.instanceMatrix.needsUpdate = true
           ~~~~~~~~~~~

src/effects/Boost.tsx:34:37 - error TS2322: Type 'null' is not assignable to type 'BufferGeometry'.

34     <instancedMesh ref={ref} args={[null, null, length]}>
                                       ~~~~

src/effects/Boost.tsx:34:43 - error TS2322: Type 'null' is not assignable to type 'Material | Material[]'.

34     <instancedMesh ref={ref} args={[null, null, length]}>
                                             ~~~~

I figure I should ask for help first before I try more files.

@vercel
Copy link

vercel bot commented Jun 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/racing-game/4Ac8gmrd9oSF3kQBWMzPJ4EsVBKi
✅ Preview: https://racing-game-git-fork-bjornstar-typescript-effects-boost-pmndrs.vercel.app

src/effects/Boost.tsx Outdated Show resolved Hide resolved
@bjornstar
Copy link
Member Author

Ok, now we're left with:

src/effects/Boost.tsx:34:37 - error TS2322: Type 'null' is not assignable to type 'BufferGeometry'.

34     <instancedMesh ref={ref} args={[null, null, length]}>
                                       ~~~~

src/effects/Boost.tsx:34:43 - error TS2322: Type 'null' is not assignable to type 'Material | Material[]'.

34     <instancedMesh ref={ref} args={[null, null, length]}>
                                             ~~~~

@Gusted
Copy link
Contributor

Gusted commented Jun 13, 2021

You're not going to like this one.

<instancedMesh ref={ref} args={[null as unknown as BufferGeometry, null as unknown as Material, length]}>

You cant cast null to a defined type, but with a little MITM of unknown you can cast it as any type.

Or you could go the easy way and use

<instancedMesh ref={ref} args={[null as any, null as any, length]}>

These type declarations of this TSX are are a bit incorrect as the instancedMesh will work without these args but still need the length.

@drcmda
Copy link
Member

drcmda commented Jun 13, 2021

yes, looks like TS types for instancedmesh are wrong, they are most definitively optional. imesh inherits from mesh:

Mesh( geometry : BufferGeometry, material : Material )
geometry  (optional) an instance of BufferGeometry. Default is a new BufferGeometry.
material  (optional) a single or an array of Material. Default is a new MeshBasicMaterial

@joshuaellis could we change these for the next @types/three release?

@joshuaellis
Copy link
Member

@drcmda hm, we have to pass count, so null wouldn't suffice as part of the args call anyway, you'd have to replace it with undefined in the args array otherwise the defaults for Mesh wouldn't activate which is why it works.

Maybe @bjornstar you could open an issue in three-ts-types and we can arrange this as a fix.

@bjornstar
Copy link
Member Author

@drcmda hm, we have to pass count, so null wouldn't suffice as part of the args call anyway, you'd have to replace it with undefined in the args array otherwise the defaults for Mesh wouldn't activate which is why it works.

Maybe @bjornstar you could open an issue in three-ts-types and we can arrange this as a fix.

Sure, I've created an issue here: three-types/three-ts-types#92

@joshuaellis
Copy link
Member

I've opened an issue in three.js as you can see above. For now I would use //@ts-expect-error and put a link to the three issue.

@bjornstar
Copy link
Member Author

I've opened an issue in three.js as you can see above. For now I would use //@ts-expect-error and put a link to the three issue.

Excellent, thank you!

@bjornstar
Copy link
Member Author

@drcmda @Gusted Thank you, this is exactly what I was hoping to see in the migration to typescript.

@bjornstar bjornstar merged commit 6ba46e6 into pmndrs:typescript Jun 13, 2021
@bjornstar bjornstar deleted the typescript-effects-boost branch June 13, 2021 10:49
Gusted pushed a commit that referenced this pull request Jun 13, 2021
* Convert Boost to tsx

* use null! in useRef

* Add comment for workaround

* Add Props and return value

* Add ts-expect-error

* ordering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants