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

TypeScript #11

Closed
ksjogo opened this issue Mar 7, 2019 · 53 comments
Closed

TypeScript #11

ksjogo opened this issue Mar 7, 2019 · 53 comments

Comments

@ksjogo
Copy link
Contributor

ksjogo commented Mar 7, 2019

Any interest in supporting TypeScript?
Initially probably only the inner workings (react-reconciler provides types) and then finding a way to generate types for the THREE bindings.

@drcmda
Copy link
Member

drcmda commented Mar 7, 2019

Sure, that would be awesome! Would you like to work on that? I am more than willing to share this lib, put it on a neutral org at some point and give out access rights. No point in just one person dealing with it. Just need to get the initial foundation right. (@bl4ckm0r3) TS is something i wouldn't be able to do myself (still need to learn it 😓)

@bl4ckm0r3
Copy link

I think it's a good idea!
Does Three have type definitions?

@ksjogo
Copy link
Contributor Author

ksjogo commented Mar 7, 2019

Started yesterday, just making it compile through babel now: https://github.com/ksjogo/react-three-fiber/tree/typescript

Yes, there are types for three as well: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/three
Afaics react-spring doesn't have any yet, though that is just for examples now.

@paulmelnikow
Copy link
Contributor

Is the goal to write the library in TypeScript, or to consume it from TypeScript? @ksjogo's branch is doing the first. Though I'm more interested in the second.

It's not clear initially how to write my own type definitions for the library. The Three-related components need to be added to in JSX.IntrinsicAttributes, though I haven't figured out how to do that.

@drcmda
Copy link
Member

drcmda commented Mar 25, 2019

I would be fine with both, though with react-spring contributed types were always lagging behind and when i made changes to the lib i didn't know how to update them (b/c i don't know tS). if the lib itself is TS i have to make do and i think it's a better experience for you all.

@4nte
Copy link

4nte commented Apr 25, 2019

It's not clear initially how to write my own type definitions for the library. The Three-related components need to be added to in JSX.IntrinsicAttributes, though I haven't figured out how to do that.

I've created a react.d.ts file and extended theIntrinsicElements interface:

declare module JSX {
    interface IntrinsicElements {
        "group": any,
        "geometry": any,
        "lineBasicMaterial": any,
        "mesh": any,
        "octahedronGeometry": any,
        "meshBasicMaterial": any,
    }
}

I did this just so that it works on my typescript project.

@4nte
Copy link

4nte commented Apr 26, 2019

Is anyone working on typescript defs atm or plans to do it?

@drcmda
Copy link
Member

drcmda commented Apr 26, 2019

Yes, #59

@sompylasar
Copy link

To make the types for the JSX elements automatically generated following what react-three-fiber does internally with lowercasing/uppercasing the component names based on THREE exports as ideated by @aleclarson in this comment, this yet non-existent feature of TypeScript is required.

@mattes3
Copy link

mattes3 commented Jun 1, 2019

To make the types for the JSX elements automatically generated following what react-three-fiber does internally with lowercasing/uppercasing the component names based on THREE exports as ideated by @aleclarson in this comment, this yet non-existent feature of TypeScript is required.

In the meantime, while we don't have that Typescript feature, we can use a work-around to consume react-three-fiber from Typescript. Just replace this code:

function Thing({ vertices, color }) {
  return (
    <group ref={ref => console.log('we have access to the instance')}>
      <line>
        <geometry
          attach="geometry"
          vertices={vertices.map(v => new THREE.Vector3(...v))}
          onUpdate={self => (self.verticesNeedUpdate = true)}
        />
        <lineBasicMaterial attach="material" color="black" />
      </line>
      <mesh 
        onClick={e => console.log('click')} 
        onPointerOver={e => console.log('hover')} 
        onPointerOut={e => console.log('unhover')}>
        <octahedronGeometry attach="geometry" />
        <meshBasicMaterial attach="material" color="peachpuff" opacity={0.5} transparent />
      </mesh>
    </group>
  )
}

by that code

const e = React.createElement;

function Thing({ vertices }: { vertices: [number, number, number][] }) {
    return (
        e('group', { ref: ref => console.log('we have access to the instance') },
            e('line', null,
                e('geometry', {
                    attach: 'geometry',
                    vertices: vertices.map(v => new THREE.Vector3(...v)),
                    onUpdate: (self: any) => (self.verticesNeedUpdate = true),
                }),
                e('lineBasicMaterial', { attach: 'material', color: 'black' }),
            ),
            e('mesh',
                {
                    onClick: (e: any) => console.log('click'),
                    onPointerOver: (e: any) => console.log('hover'),
                    onPointerOut: (e: any) => console.log('unhover')
                },
                e('octahedronGeometry', { attach: 'geometry' }),
                e('meshBasicMaterial', { attach: 'material', color: 'peachpuff', opacity: 0.5, transparent: true })
            )
        )
    );
}

This way, we can use react-three-fiber until it becomes fully typed.

By the way: I don't understand why this great framework works at all! How does React.createElement() know what the tag name lineBasicMaterial means? In the code, I see some element called catalogue but I cannot see where it is being used by React. Can somebody enlighten me, please?

@drcmda
Copy link
Member

drcmda commented Jun 1, 2019

@mattes3 check out https://github.com/facebook/react/tree/master/packages/react-reconciler
this is what teaches react host-specifics. react is just the component definition (classes, hooks and createElement), but it has no knowledge about the host (dom, android, ios, windows, etc).

react-dom is the reconciler, it contains the native elements (div, span, ..), knowledge units and so on, and of course the dom api (append, remove,...).

likewise, this config: https://github.com/drcmda/react-three-fiber/blob/master/src/reconciler.tsx#L270-L342 teaches react about threejs. if you fill out these names you can use react for anything you want.

@mattes3
Copy link

mattes3 commented Jun 1, 2019

@drcmda Ah, that explains it! Thanks, Paul!

@RobRendell
Copy link

Is react-three-fiber currently usable in a Typescript project? I see that this issue is still open, but the above merge request has been merged and (presumably) released.

I ask because TS is reporting various type errors in react-three-fiber/types which I can't resolve in my project which I'm attempting to convert from React 15/react-three-renderer to React 16/react-three-fiber. I wanted to check if this is expected to work currently, in which case I'd keep trying and perhaps open a new issue if I can't solve it, or if it's still a work in progress, in which case I should report my problems here.

For the record, I'm getting an error that react-three-fiber/types/src/canvas.d.ts line 79 assigns field updateDefaultCamera in a CanvasProps, but the definition of interface CanvasProps in canvas.tsx doesn't declare that field.

E:\Users\Rob\workspace\gTove\node_modules\react-three-fiber\types\src\canvas.d.ts
Error:(79, 5) TS2339: Property 'updateDefaultCamera' does not exist on type 'CanvasProps'.

I'm also getting errors about the audio and line intrinsic elements that react-three-fiber is trying to add to the JSX namespace. The version of TS I'm using (3.5.3) balks at changing the types of previously-declared elements, and @types/react@16.8.23 declares those intrinsic elements in the JSX namespace already.

E:\Users\Rob\workspace\gTove\node_modules\react-three-fiber\types\three.d.ts
Error:(81, 7) TS2717: Subsequent property declarations must have the same type.  Property 'audio' must be of type 'DetailedHTMLProps<AudioHTMLAttributes<HTMLAudioElement>, HTMLAudioElement>', but here has type 'Object3DNode<Audio>'.
Error:(94, 7) TS2717: Subsequent property declarations must have the same type.  Property 'line' must be of type 'SVGProps<SVGLineElement>', but here has type 'Object3DNode<Line>'.

@drcmda
Copy link
Member

drcmda commented Jul 24, 2019

it's ongoing, but def needs help and contributions. there are workarounds for line and audio, we're using line_ for instance. but ultimately this error is with TS which treats dom types wrongly.

@bali182
Copy link

bali182 commented Jul 30, 2019

Hello, I'm a little late to the party, but is there anything you need help with on this? As far as I understood the task is to add the proper elements to IntrinsicElements. Is there some info about the full list of elements & their props / how to figure them out? I'd be happy to help if I can!

@drcmda
Copy link
Member

drcmda commented Jul 30, 2019

@bali182 i think the types are all available for threejs - but the lib is in a rough spot generally when it comes to TS atm. It wouldn't compile and there's some spots left that are still "any". if you see something that's fixable, sure, go ahead, would love to receive your pr.

@clement-buchart
Copy link

clement-buchart commented Aug 10, 2019

Hello,

I'm having an issue with some of the typings for the JSX elements, that I think I fixed, but would like your input before submitting a PR.

TypeScript error in /Users/cbuchart/Projects/awesomeprojectilltotallyfinishthistimeipromise/web/src/components/Sprite2D.tsx(19,50):
Type 'number[]' is not assignable to type '[Matrix4] | [Vector3] | [] | [number] | [number, number, number] | [Geometry] | [string, (event: Event) => void] | [string, (event: Event) => void] | [string, (event: Event) => void] | ... 6 more ... | undefined'.
  Property '0' is missing in type 'number[]' but required in type '[Vector3[] | Vector2[]]'.  TS2322

    17 |                 <meshBasicMaterial attach="material" map={map} args={ [{ shadowSide: DoubleSide }] } alphaTest={0.5}/>
    18 |                 <meshDepthMaterial attach="customDepthMaterial" depthPacking={RGBADepthPacking} alphaTest={0.5}/>
  > 19 |                 <planeGeometry attach="geometry" args={ [5,5,10,10] } />
       |                                                  ^
    20 |             </mesh>
    21 |         </group>
    22 |     )

After investigation planeGeometry is a ReactThreeFiber.GeometryNode<THREE.PlaneGeometry>, hence a Node<THREE.PlaneGeometry>,which means it's args props is a Args<THREE.PlaneGeometry>, ie a ConstructorParameters<ClassSignature<THREE.PlaneGeometry>>, where ClassSignature<T> seems to extract the constructor type of T (which I'm not sure is possible since it's become a type parameter and not the full class anymore);

Anyway, here is the fix that worked for me : I changed planeGeometry to be a ReactThreeFiber.GeometryNode<typeof THREE.PlaneGeometry> (typeof SomeClass giving the type of SomeClass constructor in Typescript), and Node to be

type Node<T, P = ConstructorParameters<T>> = Overwrite<
        Partial<InstanceType<T>>,
        {
            /** Using the attach property objects bind automatically to their parent and are taken off it once they unmount. */
            attach?: string
            /** Constructor arguments */
            args?: P
            children?: React.ReactNode
            ref?: React.Ref<React.ReactNode>
            key?: React.Key
        }
        >

(ie, I use typescript built-in ConstructorParameters<T> to get the constructor parameters for the args field, and the other built-in InstanceType<T> the get the fields of THREE.PlaneGeometry)

Then, the args of my planeGeometry is [(number | undefined)?, (number | undefined)?, (number | undefined)?, (number | undefined)?] | undefined as I would expect.

Obviously the typeof must be added for all types in IntrinsicElements.

Let me know if that sounds correct to you, and I'll submit a PR.

Cheers

@alecmolloy
Copy link

it's ongoing, but def needs help and contributions. there are workarounds for line and audio, we're using line_ for instance. but ultimately this error is with TS which treats dom types wrongly.

want to start out by thanking you for all your hard work here! this is an amazing library, and its exciting to get in on this with so much moving. :)

i'm getting the same errors with audio, but am more concerned about this clash of the JSX DOM names, what happens if i want to use an HTML DOM element in a project as well? It feels like this is something I should be able to do and the clash is a bit confusing.

@drcmda
Copy link
Member

drcmda commented Aug 11, 2019

the only elements i am aware of that cause problems are line, and audio. and the error is with typescript. there is nothing we can do other than hoping microsoft will fix it. jsx is a cross platform standard, it has no relation whatsoever with the dom. that they assume dom elements in the jsx intrinsic types is really weird. there are couple of issues open in the ts issue tracker requesting a better solution. maybe if someone knew how to phrase that we could try to bring it to the attention of the react team.

but as i said, in the meantime use line_/audio_ and it's fine. you can of course mix dom elements and three elements inside of their respective reconcilers.

import { ReactThreeFiber } from 'react-three-fiber/types/three'

declare global {
  // tslint:disable-next-line: no-namespace
  namespace JSX {
    // tslint:disable-next-line: interface-name
    interface IntrinsicElements {
      line_: ReactThreeFiber.Object3DNode<THREE.Line>
      audio_: ReactThreeFiber.Object3DNode<THREE.Audio>
    }
  }
}

@alecmolloy
Copy link

alright then. thanks for explaining, and appreciate the code example! looking at submitting a PR of updates to the types in the next few days, will check and see what people are doing.

@drcmda
Copy link
Member

drcmda commented Aug 12, 2019

made some progress today, the lib finally runs through TS without errors: #168 there are a couple of any's left in the reconciler but i doubt they can be fixed since it can basically accept any object given to it via extend.

@alecmolloy
Copy link

wow! this is fantastic. going to help so much with my project i'm working on. thanks for taking this on. :)

@promontis
Copy link

@drcmda ah awesome work! Is this in the typescript branch? Any idea when it will be merged to master and pushed to npm?

@drcmda
Copy link
Member

drcmda commented Aug 12, 2019

hope tomorrow, i'll do some more testing in case i broke something.

@drcmda
Copy link
Member

drcmda commented Aug 15, 2019

So when the next PR is merged, this will work automatically without extra imports?

@dm385
Copy link
Contributor

dm385 commented Aug 15, 2019

So when the next PR is merged, this will work automatically without extra imports?

Yes, the extra import should not be needed any more after #170

@RobRendell
Copy link

With dependencies of "react-three-fiber": "^2.3.6" and "three": "^0.107.0" in my package.json, I'm still getting the same conflict between the types of the two libraries' JSX.IntrinsicElements.audio and JSX.IntrinsicElements.line declarations ("Subsequent property declarations must have the same type.")

@drcmda - when you said that react-three-fiber@^2.3.0 "runs through TS without errors", how did you resolve this type conflict? I'm presumably doing something wrong...

@dm385
Copy link
Contributor

dm385 commented Aug 20, 2019

@RobRendell please, take a look at #172

@HuiGeGeGitHub
Copy link

@RobRendell I add configuration to tsconfig.json, generally we don't need to verify node-module,This works fine.
"exclude": [ "node_modules" ]

@hasparus
Copy link
Contributor

@ksjogo it took some time, but react-three-fiber v4 should work with TypeScript pretty well :)

@etienne-85
Copy link

Hello!
I just started using three-fiber and I'am porting some of my previous work to it using TS
I copied code from a fiber demo to use OrbitControls and I get this error "Property 'orbitControls' does not exist on type 'JSX.IntrinsicElements'"
Do we need to write type definitions for extended modules such as OrbitControls?

@drcmda
Copy link
Member

drcmda commented Feb 7, 2020

yes, they're not part of three/types

import { ReactThreeFiber } from 'react-three-fiber'
import { OrbitControls } from 'three/examples/jsm/controls/OrbitControls'

declare global {
  namespace JSX {
    interface IntrinsicElements {
      orbitControls: ReactThreeFiber.Object3DNode<OrbitControls, typeof OrbitControls>
    }
  }
}

@micha149
Copy link
Contributor

Is the array short hand syntax supported yet?

I have the following line in my code:

<polarGridHelper args={[ 200, 16, 16, 64, 0x333333, 0x333333 ]} rotation={[Math.PI / 2, 0 ,0]} />

and typescript reports that number[] is missing properties from type Euler on property rotation.

I think problem is that the corresponding codelines are only covered by the properties type from (inherited) Object3D and need to be something like Object3D['rotation']|Parameters<Object3D['rotation']['set']> (or more specifically their generics).

Or am I doing it wrong?

@micha149
Copy link
Contributor

Ah, I digged a little bit deeper. I think the PolarGridHelper got a wrong type in three-types.ts. The helper seems to inherit from Object3D but its defined via ReactThreeFiber.Node instead of ReactThreeFiber.Object3DNode.

Is this file generated? I would do a PR.

I would also suggest to improve definitions of Euler, Vector3, etc. with Properties utility Type, like mentioned above.

@hasparus
Copy link
Contributor

hasparus commented Feb 27, 2020

hey @micha149, three-types.ts is handwritten. The only thing that is generated is components/generated.ts

Could you check if the type of PolarGridHelper from react-three-fiber/components is correct?

Edit:

I'll just note a workaround here. As we wait for micha149's PR 😉, PolarGridHelper can be imported from generated react-three-fiber/components with correct types.

import { PolarGridHelper } from 'react-three-fiber/components';

// React.FC<ReactThreeFiber.Object3DNode<PolarGridHelper, typeof PolarGridHelper>>
const _ = <PolarGridHelper />;

Proof by CodeSandbox: https://codesandbox.io/s/modest-flower-k63fx?fontsize=14&hidenavigation=1&theme=dark

@micha149 micha149 mentioned this issue Feb 27, 2020
@drcmda
Copy link
Member

drcmda commented Feb 27, 2020

@hasparus can the typedef file be autogenerated perhaps?

@hasparus
Copy link
Contributor

@hasparus can the typedef file be autogenerated perhaps?

I kinda assumed that three-types.ts is more correct than my codegened types, because it's older 😄

We're generating this big stupid file (generated.ts) which has the strings we're interested in asserted as types of components to provide TS support.

If we'd remove React.FC here __ThreeFiberComponents turns into __ThreeFiberProps.

https://github.com/react-spring/react-three-fiber/blob/0d694ce302f70af0d9d21319dca72fda2b278c48/src/components/types.ts#L24-L36

Our IntrinsicElements interface is pretty similar to generated.ts. We could generate it.

And ince IntrinsicElements is merged (and global, which is the root cause of my problem with @types/react conflict) we can then open it once again and merge native elements added by r3f.

interface IntrinsicElements {
  primitive: { object: any,  [properties: string]: any } 
  new: { object: any,  args: any[], [properties: string]: any }
}

^btw this is a nice argument for lying about the types like React Native. IntrinsicElements is just so much underpowered compared to NewProps<T>


I have two worries/questions I'd have to investigate.

  1. generated.ts is 250 lines. One component per line. IntrinsicElements is 220 lines. Is something missing from IntrinsicElements or are we generating too much?
  2. Why is BufferAttribute explictly removed from ThreeFiberComponents, while it is present in InstrinsicElements?
    https://github.com/react-spring/react-three-fiber/blob/83cf472c24843b9686fb293fe9d15837859d87fe/src/three-types.ts#L287
    https://github.com/react-spring/react-three-fiber/blob/0d694ce302f70af0d9d21319dca72fda2b278c48/src/components/types.ts#L22
    I have added it back (removed | THREE.BufferAttribute) and both typecheck and the tests pass. This doesn't exactly paint me as a good developer because I am apparently the only person who changed that line according to git 🤣

Note: The last two lines of generateExports.ts should probably be changed to

console.log(execSync(`yarn prettier --write ${outPath}`, { encoding: 'utf-8', stdio: 'pipe' }))
console.log(execSync(`./node_modules/.bin/eslint --fix ${outPath}`, { encoding: 'utf-8', stdio: 'pipe' }))

@micha149
Copy link
Contributor

A complete generation can become difficult, if not impossible. For example the situation where you can pass an array for position and the appropriate setter on the Vector3 will be called with the array elements as arguments. This if there is a setter the type can also be an array can get complicated when genericly typed.

@hasparus
Copy link
Contributor

I suppose we’d only try generating IntrinsicElements.

@ralexand56
Copy link

So disappointing that this library doesn't work with typescript:

Property '0' is missing in type 'ReactText[]' but required in type '[number, (number | undefined)?, (number | undefined)?]'.

@hasparus
Copy link
Contributor

hasparus commented May 8, 2020

Hey @ralexand56, does this error appear in one of the examples, in library code or in your code?

It means that an array can't be empty and can't be longer than 3 elements. The thing that's now glowing red also requires numbers, and ReactText is string | number. We often use implicit coercion in JavaScript, but it's an error in TS.

Would you like to share a CodeSandbox or a link to your repo in Discussions? You can tag me there. I might be able to help you.

@RobRendell
Copy link

@ralexand56 The good news is that it does work with Typescript - I managed to convert my Typescript react-three-renderer project to react-three-fiber after they released version 4. It works quite differently, with many elements initialised with an array of args rather than individual named props, but it definitely works.

@ralexand56
Copy link

Hey @ralexand56, does this error appear in one of the examples, in library code or in your code?

It means that an array can't be empty and can't be longer than 3 elements. The thing that's now glowing red also requires numbers, and ReactText is string | number. We often use implicit coercion in JavaScript, but it's an error in TS.

Would you like to share a CodeSandbox or a link to your repo in Discussions? You can tag me there. I might be able to help you.

Thanks, I'll see if I can get that up for you soon.

@micha149
Copy link
Contributor

I came across another Typescript thing. When using forwardRef I need to specify the Type of my ref. I'm not shure which Type needs to be added, because my expected Types seem not to be exported…

import { useFrame, useUpdate, Object3DNode, GeometryNode } from 'react-three-fiber';
// Module 'react-three-fiber/targets/web' has no exported member 'Object3DNode'

// ...

const Orbit = forwardRef<Object3DNode, OrbitProps>(({
    // ...
}, ref) => {
	const orbitRef = useRef<Object3DNode>();

    // ...

    const lineRef = useUpdate<GeometryNode>(geometry => {
        // ...
    });

    return (
        <group ref={ref} rotation={[ 0, inclination, 0]}>
            <line>
                <lineBasicMaterial attach="material" color={'lightgrey'} />
                <geometry attach="geometry" ref={lineRef} />
            </line>
            <group ref={orbitRef}>
                {children}
            </group>
        </group>
    )
});

@gsimone
Copy link
Member

gsimone commented Aug 18, 2020

You should use the Object3D type and import it from three, example here https://github.com/react-spring/drei/blob/master/src/useHelper.tsx

@micha149
Copy link
Contributor

Ah, ok. got it. Make sense since ref.current contains the actual object instance from threejs.

So, for Geometry I would use the threejs class, too:

import { Object3D, Geometry } from 'three';
// ...
const lineRef = useUpdate<Geometry>(geometry => {

Thanks for your help!

@gsimone gsimone closed this as completed Aug 19, 2020
@micha149
Copy link
Contributor

I'm not sure if this closes this whole issue 😅

@gsimone
Copy link
Member

gsimone commented Aug 19, 2020

I'm closing this specific issue since it's pretty dated to begin with, please open new specific issues when needed, it's easier for us to track

gsimone added a commit that referenced this issue Sep 17, 2020
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

No branches or pull requests