fix(typescript): add types to package.json, use typescript strict, assorted type fixes #86
Conversation
| "sideEffects": false, | ||
| "scripts": { | ||
| "prebuild": "rimraf dist", | ||
| "build": "rollup -c", | ||
| "prepare": "npm run build", | ||
| "prepare": "npm run build && npm run typegen", |
There was a problem hiding this comment.
prepare will now run both the build and typegen before publishing to npm
There was a problem hiding this comment.
this doesn't let me publish though, it creates the typefiles but any error before publish makes the process stop.
There was a problem hiding this comment.
ahh fair, hmmmm will think about this one and if we can swallow the exit codes in the meantime.
| "declaration": true, | ||
| "removeComments": true, | ||
| "emitDeclarationOnly": true, | ||
| "outFile": "types/index.d.ts" |
There was a problem hiding this comment.
the types/index.d.ts file is generated via the outFile property in the tsconfig.json
| @@ -4,15 +4,16 @@ | |||
| "description": "React-fiber renderer for THREE.js", | |||
| "main": "dist/index.cjs.js", | |||
| "module": "dist/index.js", | |||
| "types": "types/index.d.ts", | |||
There was a problem hiding this comment.
here is where we reference the entry point for the types file, which other npm projects will be able to understand: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html
setsun
force-pushed
the
typescript-2
branch
5 times, most recently
from
f5bee5e
to
4f15797
Compare
src/canvas.tsx
Outdated
| setManual: (takeOverRenderloop: boolean) => any | ||
| setDefaultCamera: (camera: THREE.Camera) => any | ||
| setDefaultCamera: (camera: THREE.OrthographicCamera | THREE.PerspectiveCamera) => any |
There was a problem hiding this comment.
I think something like instanceof THREE.Camera makes more sense b/c cameras can be custom in three
| state.current.manual = takeOverRenderloop | ||
| if (takeOverRenderloop) { | ||
| // In manual mode items shouldn't really be part of the internal scene which has adverse effects | ||
| // on the camera being unable to update without explicit calls to updateMatrixWorl() | ||
| state.current.scene.children.forEach(child => state.current.scene.remove(child)) | ||
| } | ||
| }, | ||
| setDefaultCamera: cam => { | ||
| setDefaultCamera: (cam: THREE.OrthographicCamera | THREE.PerspectiveCamera) => { |
src/canvas.tsx
Outdated
| @@ -193,7 +195,7 @@ export const Canvas = React.memo( | |||
| useEffect(() => { | |||
| state.current.aspect = size.width / size.height || 0 | |||
|
|
|||
| if (state.current.camera.isOrthographicCamera) { | |||
| if (state.current.camera instanceof THREE.OrthographicCamera) { | |||
There was a problem hiding this comment.
would prefer isOrthographicCamera here b/c it's the official way. the idea is that cams can be both perspective and orthographic, the flag indicates which is active
There was a problem hiding this comment.
That's fair, I think this was returning a type error because isOrthographicCamera is not a defined property on THREE.Camera or THREE.PerspectiveCamera.
This was the way I settled on to satisfy TypeScript, but the alternative might be to typecast state.current.camera like the following:
if ((state.current.camera as THREE.OrthographicCamera).isOrthographicCamera)
src/canvas.tsx
Outdated
| if (ready) { | ||
| state.current.gl.setSize(size.width, size.height) | ||
| if (state.current.camera.isOrthographicCamera) { | ||
|
|
||
| if (state.current.camera instanceof THREE.OrthographicCamera) { |
src/canvas.tsx
Outdated
| } | ||
|
|
||
| export type CanvasProps = { | ||
| children: React.ReactNode | ||
| gl: THREE.WebGLRenderer | ||
| orthographic: THREE.OrthographicCamera | THREE.PerspectiveCamera | ||
| camera?: THREE.OrthographicCamera | THREE.PerspectiveCamera |
|
Looks great so far, i'm really curious how the IntrinsicElements things is going to work out 🙂 |
|
@setsun is this ready to go in? |
|
@drcmda apologies for the delay. I'll plan on getting this PR into a mergable state by tonight. I'll most likely have a couple of follow-up PRs for the remaining items. 👍 |
|
@drcmda this chunk of work should be good to merge in. I've changed the |
Fixes
For projects to recognize the TypeScript definitions for
react-three-fiber, atypesproperty needed to be added topackage.json, so this should fix issues people have had with their projects finding the types.This also necessitated merging all of the types into a single
types/index.d.tsfile for convenience as the one entry point for all the types.I've also added
strict: trueto thetsconfig.json, which will give us many more type errors, but should be non-blocking for everyday development.This PR also includes some assorted type fixes, there are currently 90~ type errors with the new
strict: truecheck which will take some effort to address all at once.To-do
JSX.IntrinsicElementsto allow typechecking when using Three.js components like<mesh />similar to how @paulmelnikow mentioned here: feat(typescript): Basic TypeScript support #59 (comment).Suggestions to this PR are welcome, and additional help on some of these TypeScript issues are also welcome!