-
-
Notifications
You must be signed in to change notification settings - Fork 35.4k
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 Rollup Config to module examples to UMD #15526
Conversation
Nice, thanks! This change works with the current script-based usage, as in our example demos, but UMD modules are also meant to work as CommonJS modules and in that case we have this output:
I think the generated file should depend on the UMD build file instead? Or even just
I think that's preferable in any case. |
rollup-examples.config.js
Outdated
banner: | ||
'/**\n' + | ||
` * Generated from '${ path.relative( '.', inputPath.replace( /\\/, '/' ) ) }'\n` + | ||
' **/\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: typically the header would end in */
rather than **/
I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this, too! Thanks
For example, Browserify-based apps can currently do something like this: const THREE = window.THREE = require('three');
require('three/examples/js/loaders/GLTFLoader.js'); That second line appends GLTFLoader to the const THREE = require('three');
const GLTFLoader = require('three/examples/js/loaders/GLTFLoader.js'); |
Good catch -- I just updated the config so it converts the I'm less familiar with Browserify but my experience with other bundlers says you should be able to just import GLTFLoader and it will pull three in implicitly. However keep in mind that because the modules are using named exports your code will probably look like this: const { GLTFLoader } = require('three/examples/js/loaders/GLTFLoader.js'); |
^Looks good, thanks! |
Thanks! |
The script currently produces this: this.manager = ( manager !== undefined ) ? manager : three_module_js.DefaultLoadingManager; Shouldn't it stay like this? this.manager = ( manager !== undefined ) ? manager : THREE.DefaultLoadingManager; |
The UMD wrapper is pretty convoluted to read but it's the same thing. The relevant factory call in the UMD wrapper when loading with a script tag is |
For readability I suppose we could just include an inline plugin function renaming |
My thought was that the built UMD modules shouldn't even necessarily be considered read-able (and to maybe even minify the builds) to discourage editing of them directly and instead provide a banner pointing to edit the source js modules instead. Of course this means that copying and editing the UMD variants wouldn't be all that easy, which was always one of the features promoted by naming the folder "examples" in the first place. I don't think I have any strong opinions about either approach, though. I just hope we don't start getting PRs submitting fixes for the UMD files instead of the jsm ones |
We are just starting a transition. I think we should keep them readable and with the least changes possible. So I agree that we should rename |
It might prevent some confusion for people not following this discussion to include a banner at the top of the generated files saying not to edit the file directly. |
Oops, I totally missed that. But yes, perhaps just add something like "It's not recommended to edit this file directly". |
Related to #15518
Changes
This change adds a new rollup config for converting all js modules in the
example/jsm
folder into UMD equivalents inexample/js
and moves towards the module syntax being the primary way to maintain the examples. The files are converted in a way that is backwards compatible with how the example scripts are currently used so no other code changes will be needed.Rollup Output
To build the
jsm
files just runnpm run build-examples
. You can see examples of the output in the linked GLTFLoader and OrbitControls. You can verify the list of examples with the new files here.Moving forward converted modules will just need to be moved into the
jsm
folder in the same folder structure asjs
and they will be converted. One caveat of this conversion method to support backwards compatibility (extending the global THREE object in the UMD files) is that the modules must use named imports and not default ones. For exampleexport { OrbitControls };
is good whileexport default OrbitControls;
is bad.