pmndrs / drei

🥉 useful helpers for react-three-fiber
https://docs.pmnd.rs/drei
MIT License
8.21k stars 670 forks source link

Need patch to use Drei with React Bootstrap #1193

Open coobeeyon opened 1 year ago

coobeeyon commented 1 year ago

Problem description:

Drei injects IntrinsicElements globally and this creates an incompatibility with React Bootstrap because it causes types to blow up into unions too large for the TS compiler to manage. In some cases I see it adding things like Node<any, any> so I'm not entirely surprised. This code sandbox shows a minimal repro: https://codesandbox.io/s/ts-compiler-error-repro-2xpkz0?file=/src/App.tsx

Relevant code:

This code sandbox shows a minimal repro: https://codesandbox.io/s/ts-compiler-error-repro-2xpkz0?file=/src/App.tsx

import "@react-three/drei";
import { Button} from "react-bootstrap";
export default function App() {
  return <Button />;
}

Suggested solution:

I think that if this injection is absolutely necessary the problem can be avoided by making sure that the new definitions of IntrinsicElements are much narrower in the types that they use. This would be a real solution, not a workaround.

This can be worked around however with the attached patch which removes all global injections: @react-three+drei+9.47.1.zip

I think that if this injection is absolutely necessary the problem can be avoided by making sure that the new definitions of IntrinsicElements are much narrower in the types that they use. This would be a real solution, not a workaround.

drcmda commented 1 year ago

I think you are just missing @types/three. Please install it. There should be nothing wrong with the intrinsic types, they’re just typing out the objects.

coobeeyon commented 1 year ago

That doees not fix the problem either locally or in the sandbox: https://codesandbox.io/s/ts-compiler-error-repro-with-types-three-ze2heb?file=/src/App.tsx

drcmda commented 1 year ago

it is still not clear to me what the problem is, that is, if it's even ours or bootstraps, or typescripts. what does boostrap do with our types, and why? atm we just type our native elements, wich imo is not something i'd want to remove just like that if it can be avoided or remedied.

@CodyJasonBennett is this something we can fix? i saw you don't write into global intrinsic space no more, but in drei we still do, is that what's causing it?

CodyJasonBennett commented 1 year ago

We can try writing into ThreeElements in a major, but I think this would need a significant refactor to narrow everything on top of pending types work (related #823, #1075).

coobeeyon commented 1 year ago

I had the feeling that some of the injections were not causing problems, but didn't do an exhaustive search. This could mean that injecting narrower types could fix the problem until you have time to make the big switch.

uhgtg commented 1 year ago

Hey Have you already solved the problem? Because I also have it.

coobeeyon commented 1 year ago

The patch I posted above is still working for us here as a workaround.

uhgtg commented 1 year ago

@coobeeyon can you please explain how I can apply the patch

coobeeyon commented 1 year ago

This is the method I use: https://dev.to/zhnedyalkow/the-easiest-way-to-patch-your-npm-package-4ece

P4tr0ll commented 1 year ago

Same problem, for now I'm gonna use Bootstrap classes

<button className="btn btn-primary">text</button>

gucolin commented 10 months ago

Still seeing this issue in 9.81.0. Applied the patch attached and still getting the same error for Buttons

UPDATE: Follow the 9.47.1 patch provided above and removes all additional global injections from newer components, and worked

CodyJasonBennett commented 9 months ago

Related #823.

@gucolin, did you refactor Drei components to use ThreeElements as per https://github.com/pmndrs/drei/issues/1193#issuecomment-1363320267 or simply remove them?

xcibe95x commented 8 months ago

Encountered the same issue today, i updated to latest and patched it myself, i had a few issues with a dropdown from react-bootstrap still, but after wrapping it in a JSX fragment the error randomly went away? idk. Anyways. here's the latest patch if someone needs: @react-three+drei+9.92.5.patch

gucolin commented 8 months ago

Related #823.

@gucolin, did you refactor Drei components to use ThreeElements as per #1193 (comment) or simply remove them?

I just removed them.