BabylonJS / Babylon.js

Babylon.js is a powerful, beautiful, simple, and open game and rendering engine packed into a friendly JavaScript framework.
http://www.babylonjs.com
Apache License 2.0
22.76k stars 3.39k forks source link

@babylonjs/inspector and gui-editor packages install unnecessary dependencies / cause TS errors in certain project setups #13581

Open wmurphyrd opened 1 year ago

wmurphyrd commented 1 year ago

Repro

npm 8.19.3

Additional context

Installing react types creates big problems in projects using typescript and non-react frameworks like vue. See e.g. vuejs/language-tools#592

These package are distributed as a pre-built bundles with the main and module fields pointing to dist files, so there's no need to include build-time dependencies, but npm installs these because they are listed as peer dependencies: https://github.com/BabylonJS/Babylon.js/blob/d521fd0ce8299552f1ea67c2a2198907f50507a5/packages/public/%40babylonjs/inspector/package.json#L24-L33

Moving both @types/react and @types/react-dom from peerDependencies to devDependencies in these two packages would resolve the issue

RaananW commented 1 year ago

The inspector typings depend on react, so those typings are needed in order to use the inspector as is. Building with the inspector will fail on (ts-enabled) projects that don't include those. npm has changed the way it treats peer dependencies (personally I find it better, but one could argue that it isn't ;-) ). So the dependency is installed. I'll be happy to discuss it in the forum. i'll be happy to understand in what project this files and why. i'm closing it for now (as we are using github issues exlusively to track work) and waiting for a ping on our forum.

wmurphyrd commented 1 year ago

@RaananW types don't exist at runtime in javascript, so having type libraries as a runtime dependency is never useful

I'll leave this here for others that have the issue then since this is closed: the workaround described here for stubbing out the unnecessary react types library does work: https://github.com/vuejs/language-tools/discussions/592#discussioncomment-1763880

RaananW commented 1 year ago

You will notice i did mention typescript in my answer. And the same goes to what you said - it shouldn't fail 🙃 But i do want to find a solution that works for everyone and will be really happy to discuss it longer in our forum, if possible

RaananW commented 3 months ago

I did some research regarding what can be done here. I believe the es6 version could be exported without the react dependencies, if we change the public API available for the package consumer. However, I prefer not to make this change a week before our 7.0 release. I am moving this to 8.0 for now.

github-actions[bot] commented 2 months ago

This issue has been automatically staled because it has been inactive for more than 14 days. Please update to "unstale".