Conflict with @types/react svg types in JSX.IntrinsicElements

[hasparus]

Hi! I've encountered a little problem while using react-three-fiber in TypeScript.
@types/react conflicts with three-types.d.ts on global JSX.IntrinsicElements.

Is there a workaround to avoid it?

(property) JSX.IntrinsicElements.line: React.SVGProps<SVGLineElement>
Subsequent property declarations must have the same type. 
Property 'line' must be of type 'SVGProps<SVGLineElement>',
but here has type 'Object3DNode<Line, typeof Line>'.
ts(2717)

---

Repro: https://codesandbox.io/s/react-three-fiber-types-conflict-repro-ydr2t
(CodeSandbox seems to be few TS versions behind so there's another problem there -- Omit type is not defined, and because of it Object3DNode is any. The conflict I mentioned still occurs regardless of it.)

[drcmda]

unfortunately this is a TS bug, there are issues on their GH already, but microsoft keeps sticking dom types into the generic jsx definitions. i believe they think jsx is for the dom only.

main issue: DefinitelyTyped/DefinitelyTyped#24433

microsoft/TypeScript#15266

microsoft/TypeScript#12754

at work we have resorted to a really ugly workaround, we use line_ instead of line for instance:

import * as THREE from 'three'
import { ReactThreeFiber } from 'react-three-fiber'

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

and then

import * as THREE from 'three'
import { Canvas, extend } from 'react-three-fiber'

extend({ Line_: THREE.Line })

...

<Canvas>
  <line_ geometry={...} />
</Canvas>

[drcmda]

Got some valuable input on Twitter https://twitter.com/0xca0a/status/1163035106472341505?s=19

[hasparus]

Thanks @drcmda! This is way better than my workaround :)

const t = {
  line: ('line' as any) as ((
    _: ReactThreeFiber.Object3DNode<THREE.Line, typeof THREE.Line>
  ) => JSX.Element),
};

function Thing({ vertices }) {
  return (
      <t.line
        onPointerDown={e => console.log(e)}
        onUpdate={line => console.log(line)}
      >
        <geometry
          attach="geometry"
          vertices={vertices.map(v => new THREE.Vector3(...v))}
          onUpdate={self => (self.verticesNeedUpdate = true)}
        />
        <lineBasicMaterial attach="material" color="black" />
      </t.line>
  );
}

Well, JSXAttributes isn't the best thing about TypeScript TBH.

[hasparus]

Okay, so according to what Sebastian Markbåge said about what React Native is doing, lying about the type is a recommended solution.

With dynamic runtime eval (/ generated wrappers) the types would probably go somewhere in this direction 🙈

type ThreeFiberJsx = {
  [P in keyof typeof THREE]: (typeof THREE)[P] extends (new () => Object3D)
    ? React.FC<
        ReactThreeFiber.Object3DNode<
          InstanceType<(typeof THREE)[P]>,
          (typeof THREE)[P]
        >
      >
    : never // GeometryNodes etc
};

[hasparus]

I feel we can close this issue -- it overlaps with #11 .

[drcmda]

i reopened b/c that would be a feasible thing we could do. maybe a 3.x milestone? im still on vacation, but let's just keep this around, it could solve all our troubles. :-)

[drcmda]

we can continue here: #189

[codynova]

@drcmda Can you help me understand this proposed workaround? Is this meant to overwrite the typings that live in react-three-fiber/types/src/three-types.d.ts? I can't seem to get it working even with this declaration in my app entry file. I should note that I'm seeing the "Subsequent property declarations must have the same type." errors being emitted from three-types.d.ts without actually attempting to import or use line or audio.

import * as THREE from 'three'
import { ReactThreeFiber } from 'react-three-fiber'

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

and then

import * as THREE from 'three'
import { Canvas, extend } from 'react-three-fiber'

extend({ Line_: THREE.Line })

...

<Canvas>
  <line_ geometry={...} />
</Canvas>

[drcmda]

this is what i saw in our project at work, but i don't know ts deep enough to know how it works. they seem to override these type. the extend function is clear but not the rest. i think better ask on spectrum, someone probably knows it.

[hasparus]

@codynova
This is half of the workaround. The first snippet is doing declaration merging, so you TypeScript knows about extend({ Line_: THREE.Line }) and use line_ with typechecking and autocomplete.

To get rid of errros you need to remove conflicting types from three-types.d.ts (audio and line) and patch package this file.

---

i think better ask on spectrum, someone probably knows it

Sorry for answering here, but I couldn't find @codynova's question on Spectrum 😅.

[drcmda]

we didnt use patch-package though, we have a file called three-fiber.ts with the following contents:

import * as THREE from 'three'
import { ReactThreeFiber } from 'react-three-fiber'

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

we import it in the index.js, i guess after three-fiber makes its own declarations, and that overrides it just fine.

[hasparus]

@drcmda Do you have skipLibCheck option in your tsconfig.json set to true? Since this conflict is entirely in node_modules, this could do the trick and ignore the error.

I've updated my repro repo to new packages version and I'm still getting the error.
TSC output below:

> yarn tsc --noEmit
yarn run v1.12.3
$ /workspace/react-three-fiber-types-conflict-repro/node_modules/.bin/tsc --noEmit
node_modules/react-three-fiber/types/src/three-types.d.ts:64:7 - error 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, typeof Audio>'.

64       audio: ReactThreeFiber.Object3DNode<THREE.Audio, typeof THREE.Audio>
         ~~~~~

  node_modules/@types/react/index.d.ts:2839:13
    2839             audio: React.DetailedHTMLProps<React.AudioHTMLAttributes<HTMLAudioElement>, HTMLAudioElement>;
                     ~~~~~
    'audio' was also declared here.

node_modules/react-three-fiber/types/src/three-types.d.ts:76:7 - error TS2717: Subsequent property declarations must have the same type.  Property 'line' must be of type 'SVGProps<SVGLineElement>', but here has type 'Object3DNode<Line, typeof Line>'.    

76       line: ReactThreeFiber.Object3DNode<THREE.Line, typeof THREE.Line>
         ~~~~

  node_modules/@types/react/index.d.ts:2990:13
    2990             line: React.SVGProps<SVGLineElement>;
                     ~~~~
    'line' was also declared here.


Found 2 errors.

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
/workspace/react-three-fiber-types-conflict-repro ���

> yarn tsc --noEmit --skipLibCheck
yarn run v1.12.3
warning package.json: No license field
$ /workspace/react-three-fiber-types-conflict-repro/node_modules/.bin/tsc --noEmit --skipLibCheck
Done in 2.74s.

[drcmda]

this is our config, i didn't make it so im just posting it here:

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "lib": ["es5", "es2015.promise", "es2016", "dom"],
    "outDir": "build/",
    "baseUrl": "./",
    "strict": true,
    "allowJs": false,
    "declaration": true,
    "preserveConstEnums": true,
    "sourceMap": true,
    "skipLibCheck": true,
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "resolveJsonModule": true,
    "noImplicitAny": true,
    "strictFunctionTypes": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "jsx": "react"
  },
  "include": ["./src/**/*"]
}

[codynova]

@hasparus That must be it. If skipLibCheck is not true, the lib types are checked before the app entry is reached, so the declaration merging has no effect. I think necessitating skipLibCheck is very unfortunate, but I understand this is a Microsoft typings issue... Sounds like patch-package may be able to solve the problem in the interim without skipLibCheck though? I will test

[hasparus]

@codynova

skipLibCheck has no effect on declaration merging here. TypeScript just
stops checking if your libraries fit together properly. It skips type
checking of library types. The conflict between react-three-fiber and
@types/react still exists, you just say in your tsconfig „well, it’s not my
problem”.

Notice that we only add line_ during declaration merging. We do nothing
with line.

[codynova]

@hasparus Right, I think we are maybe saying the same thing. But, is there any reason not to just use patch-package to update the type-declarations in three-types.d.ts from line -> line_? Or is that not the same thing?

[hasparus]

@codynova Yeah and after you do this you'd have to add extend({ line_: THREE.Line }) to your code or the patch-packaged react-3-fiber to map line_ to THREE.Line.

I'll try to do a PR with React Native inspired (#172 (comment)) fix for this issue.

EDIT: Actually if I'm right about how react-three-fiber dynamic runtime eval works, instead of using extend you could monkey patch (THREE as any).Line_ = THREE.Line 😆

[codynova]

@hasparus If you make that PR, I will post a boilerplate react-three-fiber, TypeScript, SCSS modules, hot module reloading repo this weekend with leading-edge versions of all dependencies.

[hasparus]

@codynova I'm afraid you'd have to wait to next weekend for my PR.

[codynova]

No rush. I'm using patch-package for now. Just @ me whenever

[axelboc]

I used to import Line from src/components/generated.ts, but the file was removed in v6. What's the new recommendation for dealing with this issue? I haven't been able to work something out from the previous comments here, unfortunately.

[drcmda]

#1152