JSM: Update rollup examples config to merge and convert files #16920

gkjohnson · 2019-06-27T01:51:15Z

This PR updates the build-examples script to provide the ability to specify which files will be merged into a single UMD file in the examples/js/ directory from examples/jsm/. At the moment only EffectComposer and Pass are merged into a single file but it can be extended to merge other files including /nodes once #16917 is merged:

var mergedFiles = [{
  paths: ['nodes/**/*.js'],
  input: 'nodes/Nodes.js',
  output: 'nodes/Nodes.js',
}, {
  paths: ['curves/**/*.js'],
  input: 'curves/index.js',
  output: 'curves/index.js',
},
// ...
];

Every other file is converted as-is with dependencies being considered external.

You can see the output of the conversion here and the diff here. Looking at what isn't generated it seems like WebGL.js, loader/ctm/CTMWorker.js, offscreen/*, vr/*, and renderers/RaytracingWorker.js are the only other files that need to be converted to modules.

I haven't had a chance to test everything yet but looking at the files it looks correct. It may be worth reverting to the pre-module versions of the example files to test that these new version work as expected.

@Mugen87 @donmccurdy

Mugen87 · 2019-06-28T18:24:28Z

It may be worth reverting to the pre-module versions of the example files to test that these new version work as expected.

I've done this today and found no issues related to this PR. To be more precise I've checked out 8d8ff4d (the commit before the first HTML files were migrated to modules) and then run your script.

The OBJ2 examples break but only because the JSM version was temporarily different than the "normal" version in the js directory. The EXR example was also broken but only because the JSM version was not up to date (missing setType() method).

So it seems the script produces a usable output 👍. The only thing that bothers me a bit is the fact that the linter complains about the wrong code styles in the generated files. I don't think this blocks the PR in any form but it would be nice if the files are formatted according to mdcs.

gkjohnson · 2019-06-28T18:53:01Z

I've done this today and found no issues related to this PR.

Woo! Thanks.

The only thing that bothers me a bit is the fact that the linter complains about the wrong code styles in the generated files.

Yeah unfortunately I'm not sure how much control control we have over the UMD require block that gets generated:

(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('three')) :
typeof define === 'function' && define.amd ? define(['exports', 'three'], factory) :
(global = global || self, factory(global.THREE = global.THREE || {}, global.THREE));
}(this, function (exports, THREE) { 'use strict';
// ...
})();

But we do have control over the indentation. I've disabled the extra indentation to make the diff easier to look at and files easier to edit but we can re-enable the added indentation so the linter doesn't spew so much by removing this line:

indent: false,

Once modules are in place I think it would be great to get the linter running over the examples/jsm files, as well.

gkjohnson · 2019-06-29T21:15:08Z

It also occurred to me that we could run eslint --fix on the generated UMD files to clean up some of the errors, as well.

donmccurdy · 2019-06-30T03:38:13Z

Once modules are in place I think it would be great to get the linter running over the examples/jsm files...

Definitely. 👍

The only thing that bothers me a bit is the fact that the linter complains about the wrong code styles in the generated files.

Just a thought, but maybe the UMD files should be minified, and omitted from the ESLint run? That would eliminate most confusion about which files to edit in PRs...

Mugen87 · 2019-06-30T09:24:54Z

Um, I think it's more user-friendly when the UMDs are not minified. This makes debugging easier which is an important factor. Especially when we include these files in fiddles or codepens.

donmccurdy · 2019-07-01T03:38:04Z

Ah, yeah the debugging point is a good one.

For fiddles and codepens I think we can encourage people to use ES modules maybe, but there are lots of developers using CommonJS in build systems like Browserify still and that's fine.

Mugen87 · 2019-07-10T16:13:54Z

I've checked the script with NodeMaterial and OBJLoader2 again and it seems the output is not usable. Right now, it tries to create UMDs for all files like ResolutionNode. However, this file depends on Vector2Node which is not in the THREE namespace. E.g. the ctor of ResolutionNode looks like so:

Vector2Node.call( this );

this.size = new Vector2();

The script creates this:

Vector2Node_js.Vector2Node.call( this );

this.size = new THREE.Vector2();

Maybe it's better to exclude NodeMaterial and OBJLoader2 for now. Or you try to bundle everything in a single file if possible.

gkjohnson · 2019-07-10T16:27:57Z

I can test those two files maybe tomorrow or Friday -- for nodes my expectation is that they would all be bundled together into a single file:

{
  paths: ['nodes/**/*.js'],
  input: 'nodes/Nodes.js',
  output: 'nodes/Nodes.js',
}

I know that nodes were originally using imports but was OBJLoader originally a module? Or hand converted from the THREE examples? Why is it different?

Mugen87 · 2019-07-10T16:34:08Z

Or hand converted from the THREE examples?

This one. The old code was deleted.

Mugen87 · 2019-07-10T16:48:21Z

Um wait. I've had a closer loo to other files now. ColladaLoader has a dependecy to TGALoader. The script produces this output in the UMD:

if ( TGALoader_js.TGALoader ) {

which does not seem to be correct. It should be

if ( THREE.TGALoader ) {

Or not? When you include the UMD version of TGALoader, the class is available in the THREE namespace, right?

gkjohnson · 2019-07-10T17:02:56Z

Oh interesting a change I made to this function may have broken it because it was definitely working before:

function resolveGlobalObject( depPath ) {

 	if ( isLibrary( depPath ) ) {

 		return 'window';

 	} else {

 		return 'THREE';

 	}

 }

I'll do more testing.

gkjohnson · 2019-07-11T03:38:24Z

The script produces this output in the UMD:
if ( TGALoader_js.TGALoader ) {
which does not seem to be correct. It should be
if ( THREE.TGALoader ) {

To elaborate on this a bit more this is actually correct and expected given the way that rollup converts files. The files are converted to the UMD format, which supports require statements (which weren't supported before) and global script tags. Take the ColladaLoader, for example, which imports THREE and TGALoader:

import { TGALoader } from './TGALoader.js';
import { /* ... */ } from '../../../builds/three.module.js';

Rollup passes each of these imports in as a separate object so when using require it calls the ColladaLoader definition function with the following arguments (the first argument being the exported object that ColladaLoader will be added on to):

// UMD
(exports, require('three'), require('..\TGALoader.js'))

When loading in a global script context, though, we've specified that all the exports should be loaded from the global THREE object so instead it calls the function like so:

// global script
(global.THREE = global.THREE || {}, global.THREE, global.THREE)

The function that's getting called has a signature that looks like this:

function (exports, THREE, TGALoader_js) {

The THREE and TGALoader_js variables names cannot be the same because it would break UMD loading. The THREE name in this case was originally three_module_js but is regex replaced here to maintain the old THREE naming.

I know it's a bit convoluted but I hope that explanation was clear. I haven't looked into a way to share the THREE name across all imports but if it's strongly desired maybe we can take a look later, though I'd prefer to do that in a future PR.

Regarding the nodes and OBJLoader2 files -- have you actually tried to use them and found them unusable? Or did the code just look unfamiliar or have unexpected variable names?

Mugen87 · 2019-07-11T10:32:14Z

Okay, thanks for the explanation! I think the syntax is okay like that. I just wanted to know the reasons behind it.

Or did the code just look unfamiliar or have unexpected variable names?

This one. I did not had the time to refactor the examples in order to test NodeMaterial and OBJ2Loader.

Mugen87 · 2019-07-11T10:33:10Z

@mrdoob I think we should give this PR a try and see how it goes. We can then deprecated modularize.js and primarily work with the module versions of the example code.

Mugen87 · 2019-07-20T18:53:02Z

I suggest merging this at the beginning of R108 dev cycle. Then we have enough time to verify it on dev.

Mugen87 · 2019-08-04T10:46:46Z

@mrdoob Do we want to give this PR a try now? It's the last step to finalize the module conversion.

gkjohnson · 2019-08-09T15:19:47Z

Especially with all of the examples being converted to modules I think it's important that this switch be made -- I've seen a number of PRs recently that only modify the jsm files inadvertently because that's the easiest way to test them (my own included 😳). It would also be good to add early in the month before the r108 release to get more time in for testing.

If something needs more explaining or we'd prefer to change the output of the rollup config let me know and I'll be happy to discuss / investigate!

This is maybe related but are there any open PRs that are being considered that modify the js files that would need to be merged first?

mrdoob · 2019-09-14T04:13:49Z

Sorry for the delay.

So... I think the strategy I'm more comfortable with by leaving examples/js alone 😕

Use case 1 - Improve existing file, say... GLTFLoader

Update examples/js/loaders/GLTFLoader.js
Run modularize.js

Use case 2 - Add new file, say... SuperMegaControls

Add examples/jsm/controls/SuperMegaControls.js

The idea is to update existing files in examples/js for backwards compatibility but if people want to use new controls, loaders, vr, ... then they'll have to switch to modules.

gkjohnson · 2019-09-15T19:22:49Z

I think the strategy I'm more comfortable with by leaving examples/js alone

Hmm. Can you elaborate on what exactly makes you uncomfortable? Maybe I can find a way to convert them in a more agreeable way and simplify the config. The strategy you proposed seems like it could be a bit confusing to me, as well.

If there are some examples only available as modules then new users will have to understand which examples are available where (I feel like I still see people using the old script tags but maybe that will die out soon). Contributors will also then have to understand which example file to edit and why, which won't be immediately clear.

I think the bigger reason I would like the jsm files to be canonical is that development / maintenance process for examples is really poor at the moment. All the example pages now use the jsm files which means updating an example script involves

editing the module file so you can actually see the changes in the browser
copy and pasting the changes to the /js file making sure you didn't miss anything
running modularize.js
make sure the script still works
repeat for further updates / change requests

You could edit the /js files directly but then that requires manually rerunning modularize every time to actually see the changes.

mrdoob · 2019-09-16T02:44:14Z

Hmm. Can you elaborate on what exactly makes you uncomfortable?

Basically, I was hoping the JS to JSM situation to be transitory. If we convert new JSM files to JS, then we're giving the signal that we support both modes and user will expect new JS files added. On top of the .d.ts files, maintenance will becomes pretty unfun.

(I feel like I still see people using the old script tags but maybe that will die out soon)

That's the idea.

Contributors will also then have to understand which example file to edit and why, which won't be immediately clear.

Yeah. We could also stop updating JS files and only update JSM files from now on. Maybe add a console.warn() to let people know.

Mugen87 · 2019-09-17T11:37:33Z

Okay. In this case, we can close the PR.

You could edit the /js files directly but then that requires manually rerunning modularize every time to actually see the changes.

We could consider to add a npm watch script that does this automatically as soon as you save a js file.

gkjohnson · 2019-09-18T06:28:29Z

@mrdoob

Basically, I was hoping the JS to JSM situation to be transitory. If we convert new JSM files to JS, then we're giving the signal that we support both modes and user will expect new JS files added. On top of the .d.ts files, maintenance will becomes pretty unfun.

Hmm okay it still doesn't feel quite right to remove the js modules completely especially given all the module import / resolution problems mentioned in some other threads, even if just to support the hacking / static example page with CDN-served libraries cases. Maybe I'm just feeling nostalgic, though :)

Ideally converting the jsm files to UMD would require little to no maintenance. This PR did get complicated but I think it can be simplified if some of the other levied requirements were loosened. Would it be worth proposing a different, simpler, examples rollup config?

We could also stop updating JS files and only update JSM files from now on. Maybe add a console.warn() to let people know.

If this is the plan, though, is there any timeline when you might expect we can consider the jsm modules canonical and mark the js files as deprecated? I can help add the warning to the files if you think it's time. Of course my preference is to do this sooner rather than later.

mrdoob · 2019-09-18T15:48:53Z

Hmm okay it still doesn't feel quite right to remove the js modules completely especially given all the module import / resolution problems mentioned in some other threads, even if just to support the hacking / static example page with CDN-served libraries cases. Maybe I'm just feeling nostalgic, though :)

I know what you mean. But I'm trying to make sure we don't end up in a situation were there are too many things to consider in order to advance. We can definitely keep these files for a year or so though.

Ideally converting the jsm files to UMD would require little to no maintenance. This PR did get complicated but I think it can be simplified if some of the other levied requirements were loosened. Would it be worth proposing a different, simpler, examples rollup config?

I think it's better to deprecate these files and encourage people to move to ES6 Modules. A single code path it's easier to maintain than two. And, considering the recent discussions, seems like ES6 Modules are already problematic enough 😣

If this is the plan, though, is there any timeline when you might expect we can consider the jsm modules canonical and mark the js files as deprecated? I can help add the warning to the files if you think it's time. Of course my preference is to do this sooner rather than later.

Yes, I think so. I think we can consider jsm modules canonical already and start adding the warnings to the js files.

donmccurdy · 2019-09-22T05:36:55Z

Yes, I think so. I think we can consider jsm modules canonical already and start adding the warnings to the js files.

What do you mean by "canonical"? I think I've been using "canonical" to refer to the file a contributor should edit, and from which other files are generated. Did you mean that, or just that the JSM files should be recommended to end-users?

donmccurdy · 2019-09-22T05:41:49Z

^Or do we mean that contributors should edit the JSM files, and the JS files would not be updated at all moving forward? The wording, "no longer maintained or updated", in #17544 makes it sound like that is the plan...

I'm not sure about that... if the point of keeping them is to give users time for the transition to ES modules, providing r108 GLTFLoader in the r115 three.js distribution seems like a confusing way to do so.

Mugen87 · 2019-09-22T15:04:54Z

I'm not sure about that... if the point of keeping them is to give users time for the transition to ES modules, providing r108 GLTFLoader in the r115 three.js distribution seems like a confusing way to do so.

Indeed, this will only lead to confusion.

mrdoob · 2019-09-23T18:20:18Z

I'm not sure about that... if the point of keeping them is to give users time for the transition to ES modules, providing r108 GLTFLoader in the r115 three.js distribution seems like a confusing way to do so.

Indeed! I thought a bit more about this and realised the same, specially when we change APIs in core and we update the examples too.

How about we continue using the existing examples/js files as canonical but we add a message warning the user that these version of the files will be deprecated in.. say... January 2020?

gkjohnson · 2019-09-23T18:26:34Z

@mrdoob heh I have a comment in the other issue (#17544 (comment)) but let's continue the conversation here.

How about we continue using the existing examples/js files as canonical but we add a message warning the user that these version of the files will be deprecated in.. say... January 2020?

I'd prefer to the get the modules canonical sooner but January isn't that far away, I suppose, if we're definitely going to remove them then. We'll have strip the warning when generating the modules, though, or the jsm files will include it, too. My preference would be to generate the js files from jsm like which would give us a bit more leeway.

From #17544 (comment):

What if we meet in the middle and use rollup to only generate /js versions from modules for files that already exist in the /js folder and add a warning like the above to warn users that they will be deprecated. That script should be pretty simple, will keep them working until we want to remove them with no new modules being added, and will let us edit the jsm modules directly for PRs and testing. /js examples can be generated before a release like builds are.

donmccurdy · 2019-09-23T18:49:30Z

How about we continue using the existing examples/js files as canonical but we add a message warning the user that these version of the files will be deprecated in.. say... January 2020?

What specifically would happen in January 2020?

mrdoob · 2019-09-23T19:52:34Z

What specifically would happen in January 2020?

Removal of examples/js folder...? 🙊

donmccurdy · 2019-09-23T23:24:45Z

Ok, if we think we'd be willing to remove examples/js on that timeline — or at least announce it and see what issues are raised — then I'd vote to:

Put a deprecation notice into the js/ files now.
Continue generating jsm/ from js/, stripping that notice.
Do not proceed with generating js/ from jsm/, since Jan 2020 is soon enough to make that transition unnecessary.

gkjohnson · 2019-09-24T03:14:03Z

A few months is definitely better than a year or so :) As long as there's a light at the end of the tunnel.

Put a deprecation notice into the js/ files now.

Continue generating jsm/ from js/, stripping that notice.

I updated #17544 to include both of these.

gkjohnson added 8 commits June 26, 2019 17:15

Update the rollup config to support multi-file merge output

2af2af3

Don't indent for minimal changes, fix global name replace

cd3f6cc

Fix the libraries global object.

8cd373d

rename variables

47a71e4

Fix plugin transform

25b660c

skip transforming the libraries

7a6a03e

normalize the path before checking if its a library

b835907

normalize the glob path

c1dfc7b

This was referenced Jun 27, 2019

JSM: Add Canonical Modules White List #15599

Closed

Example files that still need to be converted to modules #16688

Closed

Mugen87 approved these changes Jun 29, 2019

View reviewed changes

Mugen87 added this to the r107 milestone Jul 3, 2019

Mugen87 mentioned this pull request Jul 9, 2019

Transform examples/js to support modules #9562

Closed

Mugen87 mentioned this pull request Jul 17, 2019

Unify setting postprocessing pass size #17037

Merged

Mugen87 removed this from the r107 milestone Jul 20, 2019

This was referenced Aug 9, 2019

Add support for linting example scripts #17225

Merged

ReactAreaLights do not seem to work in a module bundler #17220

Closed

mrdoob modified the milestones: r108, r109 Aug 27, 2019

gkjohnson added 2 commits September 6, 2019 10:54

Merge branch 'dev' into rollup-example-update

5c384c9

comma mistype

0f815a6

Mugen87 closed this Sep 17, 2019

This was referenced Sep 19, 2019

Linting: Lint Example Shaders PR 1 #17533

Merged

Examples: Deprecate examples/js scripts #17544

Closed

zarov mentioned this pull request Oct 8, 2019

Change the ThreeExtended source in config/prepare.js iTowns/itowns#1241

Closed

Mugen87 mentioned this pull request Oct 25, 2019

Minimap demo #17811

Closed

donmccurdy mentioned this pull request Jan 27, 2020

CameraControls: consolidate OrbitControls, MapControls, and TrackballControls #18483

Merged

gkjohnson mentioned this pull request Sep 18, 2020

Fixed broken pointerevent handling when pointerType = "pen" #20336

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSM: Update rollup examples config to merge and convert files #16920

JSM: Update rollup examples config to merge and convert files #16920

gkjohnson commented Jun 27, 2019

Mugen87 commented Jun 28, 2019 •

edited

Loading

gkjohnson commented Jun 28, 2019

gkjohnson commented Jun 29, 2019 •

edited

Loading

donmccurdy commented Jun 30, 2019

Mugen87 commented Jun 30, 2019

donmccurdy commented Jul 1, 2019

Mugen87 commented Jul 10, 2019 •

edited

Loading

gkjohnson commented Jul 10, 2019

Mugen87 commented Jul 10, 2019

Mugen87 commented Jul 10, 2019 •

edited

Loading

gkjohnson commented Jul 10, 2019

gkjohnson commented Jul 11, 2019

Mugen87 commented Jul 11, 2019

Mugen87 commented Jul 11, 2019 •

edited

Loading

Mugen87 commented Jul 20, 2019

Mugen87 commented Aug 4, 2019

gkjohnson commented Aug 9, 2019

mrdoob commented Sep 14, 2019

gkjohnson commented Sep 15, 2019

mrdoob commented Sep 16, 2019

Mugen87 commented Sep 17, 2019

gkjohnson commented Sep 18, 2019

mrdoob commented Sep 18, 2019

donmccurdy commented Sep 22, 2019

donmccurdy commented Sep 22, 2019

Mugen87 commented Sep 22, 2019 •

edited

Loading

mrdoob commented Sep 23, 2019

gkjohnson commented Sep 23, 2019

donmccurdy commented Sep 23, 2019

mrdoob commented Sep 23, 2019

donmccurdy commented Sep 23, 2019

gkjohnson commented Sep 24, 2019

JSM: Update rollup examples config to merge and convert files #16920

JSM: Update rollup examples config to merge and convert files #16920

Conversation

gkjohnson commented Jun 27, 2019

Mugen87 commented Jun 28, 2019 • edited Loading

gkjohnson commented Jun 28, 2019

gkjohnson commented Jun 29, 2019 • edited Loading

donmccurdy commented Jun 30, 2019

Mugen87 commented Jun 30, 2019

donmccurdy commented Jul 1, 2019

Mugen87 commented Jul 10, 2019 • edited Loading

gkjohnson commented Jul 10, 2019

Mugen87 commented Jul 10, 2019

Mugen87 commented Jul 10, 2019 • edited Loading

gkjohnson commented Jul 10, 2019

gkjohnson commented Jul 11, 2019

Mugen87 commented Jul 11, 2019

Mugen87 commented Jul 11, 2019 • edited Loading

Mugen87 commented Jul 20, 2019

Mugen87 commented Aug 4, 2019

gkjohnson commented Aug 9, 2019

mrdoob commented Sep 14, 2019

gkjohnson commented Sep 15, 2019

mrdoob commented Sep 16, 2019

Mugen87 commented Sep 17, 2019

gkjohnson commented Sep 18, 2019

mrdoob commented Sep 18, 2019

donmccurdy commented Sep 22, 2019

donmccurdy commented Sep 22, 2019

Mugen87 commented Sep 22, 2019 • edited Loading

mrdoob commented Sep 23, 2019

gkjohnson commented Sep 23, 2019

donmccurdy commented Sep 23, 2019

mrdoob commented Sep 23, 2019

donmccurdy commented Sep 23, 2019

gkjohnson commented Sep 24, 2019

Mugen87 commented Jun 28, 2019 •

edited

Loading

gkjohnson commented Jun 29, 2019 •

edited

Loading

Mugen87 commented Jul 10, 2019 •

edited

Loading

Mugen87 commented Jul 10, 2019 •

edited

Loading

Mugen87 commented Jul 11, 2019 •

edited

Loading

Mugen87 commented Sep 22, 2019 •

edited

Loading