v9 type export coverage

[krispya]

I started a branch to update Drei to use R3F v9. Attached is a log of all the type errors I got, which are mostly along the lines of prop types. I'd like to talk about the best way to handle this in v9, if there is already a strategy I am unaware of or if we should create one.

drei-type-errors.txt

[CodyJasonBennett]

children looks like a straight type regression, will have to look into that specifically. The ReactThreeFiber namespace and individual ClassProps were depreciated since mapped types in #2465, and narrowed extend to a single interface.

import { type ThreeElement, extend } from '@react-three/fiber'

declare module '@react-three/fiber' {
  interface ThreeElements {
    customElement: ThreeElement<typeof CustomElement>
  }
}

extend({ CustomElement })

As you pointed out earlier, specific math helpers are missing which don't have an upgrade path. I think we can break up what is now MathProps into a generic math helper or alias previous common ones like Vector3.

[krispya]

Working through this one issue at a time, ClassProps can now be done with ThreeElements['class'] or JSX.IntrinsicElements['class']. So GroupProps becomes ThreeElements['groups'].

Next is Color and Vector3 (along with other MathProps), which includes the Three definition and then the other valid data types like arrays and scalars. Maybe a good interface would be Prop.Color, Prop.Vector3, etc.?

And then I'm not sure what this was supposed to be doing.

ReactThreeFiber.Overwrite<
    ReactThreeFiber.Object3DNode<ArcballControlsImpl, typeof ArcballControlsImpl>,
    {
      target?: ReactThreeFiber.Vector3
      camera?: OrthographicCamera | PerspectiveCamera
      domElement?: HTMLElement
      regress?: boolean
      makeDefault?: boolean
      onChange?: (e?: Event) => void
      onStart?: (e?: Event) => void
      onEnd?: (e?: Event) => void
    }
  >

Maybe it is the same as:

ThreeElements['object3D'] & { /*props*/ }

[krispya]

So, digging a little deeper it looks like ReactThreeFiber.Object3DNode and ThreeElement are the same since they both add all the React and Event props. But what about ReactThreeFiber.Node? A use case I found was ReactThreeFiber.Node<EffectComposer, typeof EffectComposer> where this element doesn't use the Event props but still has the React props. Specifically, these were the props added to ReactThreeFiber.Node:

interface NodeProps<T, P> {
    attach?: AttachType;
    args?: Args<P>;
    children?: React.ReactNode;
    ref?: React.RefCallback<T> | React.RefObject<React.ReactNode> | null;
    key?: React.Key;
    onUpdate?: (self: T) => void;
}

[krispya]

Okay so my previous assumption about ThreeElement was wrong.

v8 type:

export type FlyControlsProps = ReactThreeFiber.Object3DNode<FlyControlsImpl, typeof FlyControlsImpl> & {
  onChange?: (e?: THREE.Event) => void
  domElement?: HTMLElement
  makeDefault?: boolean
}

And the computed type (sorry it is a screenshot):

And then here is the v9 type I attempted:

export type FirstPersonControlsProps = ThreeElement<typeof FirstPersonControlImpl> & {
  domElement?: HTMLElement
  makeDefault?: boolean
}

And the computed type for it:

There is a big difference, but I'm not familiar enough with the typing in total to know how much of a problem it is. I do notice though that the events actually don't get included like I expected.

[krispya]

Finally, I have found that using the declare snippet you provided completely replaces ThreeElements. For example, when I do:

declare module '@react-three/fiber' {
  interface ThreeElements {
    effectComposer: ThreeElement<typeof EffectComposer>
    renderPass: ThreeElement<typeof RenderPass>
    shaderPass: ThreeElement<typeof ShaderPass>
  }
}

ThreeElements then computes as just those elements:

Declaring directly to JSX.InstrinsicElements works though:

declare global {
  namespace JSX {
    interface IntrinsicElements {
      effectComposer: ThreeElement<typeof EffectComposer>
      renderPass: ThreeElement<typeof RenderPass>
      shaderPass: ThreeElement<typeof ShaderPass>
    }
  }
}

[krispya]

I'm sorry to keep pilling these up. I believe this is related to the children props being replaced with {}. I noticed it was happening to all the instance props, for example here is ThreeElements['spotLight'].

I tried looking through the type code for an answer but I haven't yet mastered TS wizardry to properly read it. It appears to have something to do with Mutable and line 41 of three-types.

export type ThreeElement<T extends ConstructorRepresentation> = Mutable<
  Overwrite<ElementProps<T>, Omit<InstanceProps<InstanceType<T>>, 'object'>>
>

[CodyJasonBennett]

Thanks for the deep analysis. Mutable is supposed to address #2374 for math classes but I kept it for general props. Can try stripping it back on a branch and see what's causing this behavior.

[krispya]

I narrowed it down to the MathProps type. I believe it has to do with P[K] extends infer M and when the infer fails, it falls back to {}. Removing MathProps from line 38 resolves the issue.

[krispya]

The remaining type issues for Drei can be generalized to:

MathProp types
I think we just need to decide on an API and go with it. For example MathProps['vector3'] would keep it consistent with ThreeElements.

Uses of Node and Object3DNode
Node is the base type that adds the ReactProps. Object3DNode extends Node to add EventHandlers. I think ThreeElement is supposed to handle this but it currently gives me curious results.
type ArcballControlsProps = ThreeElement<typeof ArcballControls> gives me a computed type with generic args and no properties:

And then how should we know if the type should have events attached? Is that inferred?

Declaring new intrinsic types
This currently works if you declare on JSX.IntrinsicElements, but using the ThreeElements example provided overwrites it each declare instead of merging. I'm not sure why.

There are various minor type issues but these are the big ones.

[CodyJasonBennett]

Events are inferred from a compatible raycast signature, but elements may have overloads. I'm not familiar with them, but I'd like to make optional args compatible if present.

react-three-fiber/packages/fiber/src/three-types.ts

Lines 30 to 33 in 0df10bb

	interface RaycastableRepresentation {
	raycast(raycaster: THREE.Raycaster, intersects: THREE.Intersection[]): void
	}
	type EventProps<P> = P extends RaycastableRepresentation ? Partial<EventHandlers> : {}

[krispya]

I am now getting this error all over Drei with the latest type merges:

error TS4023: Exported variable {ComponentName} has or is using name 'ReactProps' from external module "C:/dev/nt--drei/node_modules/@react-three/fiber/dist/declarations/src/three-types" but cannot be named.

microsoft/TypeScript#40718
microsoft/TypeScript#9944

I tried upgrading to the latest TypeScript but it didn't help me.

Also, I'm not sure this is an actual problem, but the tuple definitions aren't being recognized as valid for rotation:
Type '[number, number, number]' is not assignable to type 'Euler | Readonly<Euler | undefined>'.ts(2322)

Edit: Turns out any number[] fails, so this is indeed a problem.

The full error::

Type 'number[]' is not assignable to type 'Euler | Readonly<Euler | undefined>'.ts(2322)
Object3D.d.ts(69, 14): The expected type comes from property 'rotation' which is declared here on type 'Mutable<Overwrite<Partial<Overwrite<WithMathProps<Mesh<BufferGeometry, Material | Material[]>>, ReactProps<Mesh<BufferGeometry, Material | Material[]>> & Partial<...>>>, Omit<...>>>'

[krispya]

I was able to fix the above error TS4023 by exporting ReactProps, but there is no reason to make this public.

[krispya]

One more issue, ReactThreeFiber.Euler is not exported and it was not trivial to add when I went to do it myself. :(

[krispya]

Looking into this tonight, I see the culprit. Euler's set has a string in it: set( x : Float, y : Float, z : Float, order : String ) : this. And MathRepresentation doesn't allow this.

[krispya]

Good news is that preliminary testing with #2705 shows ThreeElement<typeof Class> working as expected! Our typing struggles may be coming to a close soon.

[krispya]

I did another pass through Drei. ThreeElement is working just fine as are the MathTypes. There are some additional issues/questions:

• Overwrite needs an export. It's apparently special from the usual utility types one and is used in Drei.
• useLoader is giving some type difficulty. I need to look deeper into it.
• PointMaterial uses PrimitiveProps. Is there a way to get an equivalent? Would it be ThreeElements['object3D']?
• I tried using the ThreeElements interface for defining new intrinsics, but would always error on a strict type check. ie:

declare module '@react-three/fiber' {
  interface ThreeElements {
    positionMesh: ThreeElement<typeof PositionMesh>
  }
}

[krispya]

More details about the useLoader type error:

Argument of type 'typeof CubeTextureLoader | typeof RGBELoader' is not assignable to parameter of type 'LoaderProto<unknown>'.
  Types of construct signatures are incompatible.
    Type 'new (manager?: LoadingManager | undefined) => CubeTextureLoader' is not assignable to type 'new (...args: any) => Loader<any>'.
      Construct signature return types 'CubeTextureLoader' and 'Loader<any>' are incompatible.
        The types of 'load' are incompatible between these types.
          Type '(urls: string[], onLoad?: ((texture: CubeTexture) => void) | undefined, onProgress?: ((event: ProgressEvent<EventTarget>) => void) | undefined, onError?: ((event: ErrorEvent) => void) | undefined) => CubeTexture' is not assignable to type '(url: string, onLoad?: ((result: any) => void) | undefined, onProgress?: ((event: ProgressEvent<EventTarget>) => void) | undefined, onError?: ((event: ErrorEvent) => void) | undefined) => unknown'.
            Types of parameters 'urls' and 'url' are incompatible.
              Type 'string' is not assignable to type 'string[]'.ts(2345)

[CodyJasonBennett]

Looks like the big difference in CubeTextureLoader is that its load method accepts an array of strings rather than a single string. I looked through the types first as a sanity check, but the error message seems to confirm that at the end.

We don't respect that in Loader, maybe we can via an override:

react-three-fiber/packages/fiber/src/core/hooks.tsx

Lines 10 to 17 in 111b4a5

	export interface Loader<T> extends THREE.Loader {
	load(
	url: string,
	onLoad?: (result: T) => void,
	onProgress?: (event: ProgressEvent) => void,
	onError?: (event: ErrorEvent) => void,
	): unknown
	}

[CodyJasonBennett]

Experimenting with the ThreeElements issue in #2722.

I have another for useLoader in #2723 that I've tested locally to work (still needs to be cross-checked in Drei).

[krispya]

Experimenting with the ThreeElements issue in #2722.

I determined the ThreeElements issue had to do with Drei's TypeScript version after it was working in another project of mine. After updating to 4.9.5 there is no problem. I assume the version it was on had an issue with adding to an interface by redefining it.

[kongweiying2]

@CodyJasonBennett I'm getting this type error when using these versions of packages, any ideas on how I can resolve it?

Argument of type 'typeof RGBELoader' is not assignable to parameter of type 'LoaderProto<unknown>'.
  Types of construct signatures are incompatible.
    Type 'new (manager?: LoadingManager | undefined) => RGBELoader' is not assignable to type 'new (...args: any) => Loader<any>'.
      Construct signature return types 'RGBELoader' and 'Loader<any>' are incompatible.
        The types of 'load' are incompatible between these types.
          Type '(url: string, onLoad?: ((data: DataTexture, texData: object) => void) | undefined, onProgress?: ((event: ProgressEvent<EventTarget>) => void) | undefined, onError?: ((err: unknown) => void) | undefined) => DataTexture' is not assignable to type '(url: string, onLoad?: ((result: any) => void) | undefined, onProgress?: ((event: ProgressEvent<EventTarget>) => void) | undefined, onError?: ((event: ErrorEvent) => void) | undefined) => unknown'.
            Types of parameters 'onError' and 'onError' are incompatible.
              Types of parameters 'event' and 'err' are incompatible.
                Type 'unknown' is not assignable to type 'ErrorEvent'.ts(2345)

Looks somewhat similar to this type error mentioned earlier:

#2668 (comment)

I'm using these versions of the package:

"@react-three/drei": "9.65.5"
"@react-three/fiber": "8.12.2"

And you can clone the specific commit that demonstrates this type error here:
Eusaybia/lifemap@cfc9797

[CodyJasonBennett]

We just need to release another alpha of v9. They've been fixed since on the v9 branch.

[kongweiying2]

We just need to release another alpha of v9. They've been fixed since on the v9 branch.

Right, so the type fixes aren't incorporated into the v9 alpha.2 that's available right now on npm? Upgrading from v8 to v9 alpha doesn't seem to resolve the type issues but it does change the type signature.

Argument of type 'typeof RGBELoader' is not assignable to parameter of type 'new (manager?: LoadingManager | undefined) => Loader<DataTexture>'.
  Types of construct signatures are incompatible.
    Type 'new (manager?: LoadingManager | undefined) => RGBELoader' is not assignable to type 'new (manager?: LoadingManager | undefined) => Loader<DataTexture>'.
      Construct signature return types 'RGBELoader' and 'Loader<DataTexture>' are incompatible.
        The types of 'load' are incompatible between these types.
          Type '(url: string, onLoad?: ((data: DataTexture, texData: object) => void) | undefined, onProgress?: ((event: ProgressEvent<EventTarget>) => void) | undefined, onError?: ((err: unknown) => void) | undefined) => DataTexture' is not assignable to type '(url: string, onLoad?: ((result: DataTexture) => void) | undefined, onProgress?: ((event: ProgressEvent<EventTarget>) => void) | undefined, onError?: ((event: ErrorEvent) => void) | undefined) => unknown'.
            Types of parameters 'onError' and 'onError' are incompatible.
              Types of parameters 'event' and 'err' are incompatible.
                Type 'unknown' is not assignable to type 'ErrorEvent'.ts(2345)

[CodyJasonBennett]

I would not expect an alpha release to work at all and wait until we start rolling beta releases.