-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
Add three/addons (for npm users only) #23368
Conversation
Fixed some duplicate export conflicts (for example |
Seems there's a few side effects that'll have to be ironed out (they break tree-shaking and will run/bundle regardless if you import them). More concerningly, WebGPURenderer emits a side-effect that can break camera math for WebGL (related #20283). I suppose we can move WebGPURenderer's matrix stuff to its init method as a workaround but these will need a bigger fix. |
Right.. forgot to check tree-shaking. I'll investigate, thanks for testing! |
Actually, if we don't flatten the paths in This would work, right? <script type="importmap">
{
"imports": {
"three": "https://unpkg.com/three@0.137.0/build/three.module.js",
"three/addons": "https://unpkg.com/three@0.137.0/examples/jsm/"
}
}
</script>
<script type="module">
import * as THREE from 'three';
import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';
import { VRButton } from 'three/addons/webxr/VRButton.js';
...
</script> Edit: Or I guess it needs to be |
@mrdoob
import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';
wrong, see #23368 (comment) |
Also we can't use |
lol |
This works~! ✨ <script type="importmap">
{
"imports": {
"three": "https://unpkg.com/three@0.137.0/build/three.module.js",
"three-addons/": "https://unpkg.com/three@0.137.0/examples/jsm/"
}
}
</script>
<script type="module">
import * as THREE from 'three';
import { GLTFLoader } from 'three-addons/loaders/GLTFLoader.js';
import { VRButton } from 'three-addons/webxr/VRButton.js';
...
</script> https://github.com/WICG/import-maps#packages-via-trailing-slashes (Thanks to Russell Bicknell and Justin Ridgewell for the pointers! 🙏) |
So I guess we do not need this PR? We'll have to make another script to implement that pattern in all the examples though. |
Maybe, but it also exposed some potential issues for users regarding the chevrotain module and WebGPURenderer (the latter being a bit more concerning). Is there still an issue to track tree-shaking? WebGPURenderer can break some apps as-is if ever bundled alongside WebGLRenderer despite the workaround though. |
Maybe better creating new issues for these. |
After a bit of digging, it looks like List of unshaken files (excluding webgpu + nodes)
If you want to dig into it more, here is the resulting esbuild bundle: addons-esbuild.js.zip Most of them are unshaken because of the missing |
I am going to close this PR for now since I don't have the bandwitdth to solve the tree-shaking problems in all these files. We'll go with the pattern proposed by @mrdoob, best part is that it would be exactly the same both on node and the browser 💪 import * as THREE from 'three';
import { GLTFLoader } from 'three-addons/loaders/GLTFLoader.js';
import { VRButton } from 'three-addons/webxr/VRButton.js'; I'll make a PR sometime next week. |
Is "three-addons" a new npm package, or just an alias used for examples in an import map here? |
An alias but it can be a bit misleading indeed. |
Right.. we probably want this, to avoid confusion import * as THREE from 'three';
import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';
import { VRButton } from 'three/addons/webxr/VRButton.js'; |
Related issue: #23354
Description
With this PR, npm users that rely on bundlers which have tree-shaking built in, will be able to do this:
The old
examples/jsm
imports would still work fine, and nothing is changed on that front.The main commit is 2d3d558, after that I did some cleaning up in the exports of the examples.
You can test this PR by doing: