Skip to content
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

Continue support for JS libraries alongside ES6 JSM libraries #20455

Closed
Bug-Reaper opened this issue Oct 5, 2020 · 54 comments
Closed

Continue support for JS libraries alongside ES6 JSM libraries #20455

Bug-Reaper opened this issue Oct 5, 2020 · 54 comments

Comments

@Bug-Reaper
Copy link
Contributor

Bug-Reaper commented Oct 5, 2020

Is your feature request related to a problem? Please describe.

I believe it is good library hygiene to support both module and classic static file imports.This keeps the library accessible to a wider group of developers and allows developers to use their preferred style.

Personally I really try to avoid using modules in general, I like projects that are static files with classic simple JS file includes. Maybe I'm just a weirdo but I really hate how far frameworks unnecessarily abstract you from things and how much they re-invent wheels or otherwise black-box you. I know you can use modules without any frameworks, however module includes are less intuitive then traditional JS includes, they often have failed me when trying to use them within a static file setup.

Using ES6 Modules are not ideal for every deployment, though they are certainly a welcome addition. I teach many new programmers threejs because I love the library and IMO it is a great and satisfying way to get into programming. It is a lot easier to teach people basic vanilla CSS/JS/HTML without also shoving the entire node/npm + framework stack down their throat at the same time. Static libraries are simpler to use/understand and keep the barrier to entry low here.

Stylistically, I also prefer overloading THREE with additional functionality instead of adding new named functions that float around freely. Though this is obviously preference.

Describe the solution you'd like

Perhaps I can better answer this after getting a bit more information about why the decision was made to transition to modules only, but I'll take a stab at it.

The documentation addresses that ES6 modules may not work in every situation and for those situations it suggests using a bundler such as browserify/rollup/webpack/parcel etc....

My solution would be have an automatic ES6 bundler script go through the modules in /examples/jsm to generate /examples/js non module versions. This way developers no longer need to worry about making changes in two places and can continue to enjoy using the JS non module versions and global var import style if they like.

This automatic generation of JS non module files could be done as part of the build process or be a command in the package.json someone could run manually. Though I'd opt for automatic generation.

Creating this automation or otherwise maintaining the JS non module versions of this library is something I can donate my time for. If the reasoning behind jumping to ES6 only is not just removing the need to update two parallel version of the same thing manually, I'd love to discuss other solutions to address those concerns as well.

Describe alternatives you've considered

The obvious other consideration would be to leave things as is and continue maintaining JS and JSM versions of all libraries. Though considering the announcement these are being deprecated, I find this somewhat unlikely. But I'd be happy to take on responsibility for making sure JS libraries stay up to date with their JSM counterparts manually if we decide to go this route.

Additional context

Much love to this library and everyone that contributes either in code or by reporting/discussing issues.

@gkjohnson
Copy link
Collaborator

It is a lot easier to teach people basic vanilla CSS/JS/HTML without also shoving the entire node/npm + framework stack down their throat at the same time. Static libraries keep the barrier to entry low here.

Just to clarify the example js modules as maintained in this project do not require node, npm, or any build framework to use. They can be used as statically served files just like the old global imports. They just require the es6 import syntax in order to use but that will work in all modern browsers.

@Bug-Reaper
Copy link
Contributor Author

Bug-Reaper commented Oct 5, 2020

Just to clarify the example js modules as maintained in this project do not require node, npm, or any build framework to use. They can be used as statically served files just like the old global imports. They just require the es6 import syntax in order to use but that will work in all modern browsers.

Thank you for the clarification! That is indeed a good point!
I believe:

<script type="module">

  import { OrbitControls } from 'https://unpkg.com/three@<VERSION>/examples/jsm/controls/OrbitControls.js';

  const controls = new OrbitControls();

</script>

is perhaps less intuitive and harder to understand for newcomers than:

<script src="https://unpkg.com/three@<VERSION>/examples/jsm/controls/OrbitControls.js">

Also, correct me if I'm wrong, the modules do not create globals for use in later scripts? So there would be no easy way to use THREE.js /example/jsm files outside of a module?

Personally I've had issues importing modules this way in static files even when using modern web browsers. Docs also seem to indicate using a bundler may be required for common js functionality. Though I should really experiment further with THREE.js modules specifically to get better insight here.

If we can continue to support the JS versions easily with an automated ES6 bundler script which goes through the JSM versions, I think this is worth doing.

@DefinitelyMaybe
Copy link
Contributor

wait a sec... we've still got:

<script src="path/to/local/build/three.js"></script>

as oppose to:

<script type=module src="path/to/local/build/three.module.js"></script>

The first is a static script that can be used as per the old-global-way within someone's html... right? What was the thing you couldn't do anymore after a transition to ES6?

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 5, 2020

<script type=module src="path/to/local/build/three.module.js"></script>

I don't think it makes sense to import three.module.js that way. It should be:

<script type="module">

    import * as THREE from 'https://cdn.jsdelivr.net/npm/[email protected]/build/three.module.js';

   // app code

</script>

@Bug-Reaper
Copy link
Contributor Author

Bug-Reaper commented Oct 5, 2020

The first is a static script that can be used as per the old-global-way within someone's html... right? What was the thing you couldn't do anymore after a transition to ES6?

I believe you're correct. If I understand correctly the plan is to still include a "/build/three.js" in addition to the "/build/three.module.js".

However, currently using any of the utilities in "/examples/js" folder will warn you they're deprecated and soon to be removed in favor if "/examples/jsm" ES6 module versions. Which means you'd no longer be able to use the old global style of implementation.

If we can do so easily, I believe it makes sense to continue supporting /examples/js by automatically generating them via a ES6 bundler script as part of the build process.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 5, 2020

If I understand correctly the plan is to still include a "/build/three.js" in addition to the "/build/three.module.js".

Yes. However, it's questionable if this approach does make sense. When examples/js is removed, there are only a few use cases left where three.js and three.min.js are still useful.

It would be actually beneficial to remove three.js and three.min.js since it would allow us to change the main entry point of the npm package, see #19575.

If we can do so easily, I believe it makes sense to continue supporting /examples/js by automatically generating them via a ES6 bundler script as part of the build process.

The idea is to move examples/jsm to more modern JavaScript language features like classes. Since examples/js should still work with older browser, it would be necessary to configure a new (examples) build with code transpilation features. Besides, we would still keep the duplicate code base (examples/js vs examples/jsm) which is in my point of view a bad approach. It makes maintenance more complicated.

I believe the user has to take care of ES6 to ES5 conversion if required. Same for code minification or other build-related tasks.

@devingfx
Copy link
Contributor

devingfx commented Oct 5, 2020

I believe you're correct. If I understand correctly the plan is to still include a "/build/three.js" in addition to the "/build/three.module.js".

True

The problem with filles from /examples folder is that you need to use files from /examples/js when you used /build/three.js and files from /examples/jsm when you used /build/three.module.js , aka keep consistency in loading method.

Why? Because when using module imports the main THREE object is no more a plain js object THREE = {} but instead an internal browser's module object that is sealed (not extendable), therefore, files from /examples/js that tries to write a new property in THREE object fails.

So you can't mix import * as THREE from '/build/three.module.js and THREE.WhateverExample = function() ...

One possible way is to change the name of imported lib to anything else than THREE and re-create a plain js THREE global object for examples to be written in it...

This is typically the problem of

traditional JS includes

that polutes global space naming, and because you can't modify names into the loaded file you may get errors like that...
On the other hand, with modules, the user gain the power of naming during the import and it's no more the author of the lib that choose the resulting name...

ex:

<script>
// a script you can't modify already use the name THREE
var THREE = document.getElementById('div-nb-3')
</script>

<script type="module">
import * as foo from '/build/three.module.js'

THREE.appendChild( new foo.WebGLRenderer().domElement )
</script>

@Bug-Reaper
Copy link
Contributor Author

@Mugen87 You're 100% right. If we ditch the /examples/js we might as well ditch the three.js & three.min.js as they're essentially incompatible with any of the add-on modules. Their use case would be niche and this is almost guaranteed to create confusion.

@devingfx You're right that modules have advantages and eliminates potential global name conflicts. In years of use I've never had anything conflict with the THREE global variable and I think this is an unlikely scenario but your point is technically correct.

which is in my point of view a bad approach. It makes maintenance more complicated.

I believe the user has to take care of ES6 to ES5 conversion if required. Same for code minification or other build-related tasks.

@Mugen87 Is it really that terrible to maintain a traditional js include which uses a global var in addition to a module? Many libraries support both and from what I can tell the traditional JS version is often just as popularly used as the module version counterparts. Both have advantages/disadvantages and some of that boils down to preference. Is it not good to give developers the option of using a library in a non-module context?

I'm willing to take care of creating/testing the necessary code transpilation features to automatically generate three.min.js, three.js and /examples/js from the three.module.js and /examples/jsm. After the transpilation workflow has been finessed, it may require some minimal maintenance but it != maintaining two parallel versions. For the most part code would only be needed to be updated on the module files and only occasionally would you need to fix some transpilation fuckup.

I have enough projects that rely on the traditional global syntax and includes that I'm going to be doing the work for automating the transpilation of the modules anyways. I think at the very very least we could include a command in the package.json and call it "legacy-build" which transpiles the modules into three.min.js, three.js and /examples/js which behave similar to the original files now. These files don't even have to be committed to the repo or created by default. We could also warn they're for legacy support, they're not guaranteed to work, we suggest using modules instead etc...

Realistically though I think it makes more sense keep them in the repo and simply have them be automatically generated via transpiling on build.

@DefinitelyMaybe
Copy link
Contributor

DefinitelyMaybe commented Oct 6, 2020

a command in the package.json and call it "legacy-build" which transpiles the modules

seems reasonable. wasn't babel merged in recently? so I think this might be doable as is

edit: to clarify, not to have said new command to be run by anyone except the users which want said build

@gkjohnson
Copy link
Collaborator

Is it really that terrible to maintain a traditional js include which uses a global var in addition to a module?

I think the complexity of maintaining this is being under estimated. Unfortunately I don't think it's so simple with the way examples are set up in the project.

Let's look at GLTFLoader as an example. Right now all of GLTFLoader is contained in a single file which makes it easy to include at the top of an HTML file. One of the benefits of modules is that some of the larger files can be broken out into separate files that GLTFLoader can import as dependencies. What should the built global script include look like once GLTFLoader depends on four external files some of which are shared? Will users of the built global scripts now have to include all of those example js files individually? Or will some files be bundled together which would require manually maintaining a list of files that are okay to bundle together and which are not?

I think the only really set and forget case is bundling all of the example js files into a single monolithic blob which I don't think is reasonable. I think with any of these solutions there'd be some other release and documentation overhead, as well.

Perhaps there's a better way to do it but when I tried to make a rollup build that retained backwards compatibility or at least a consistent structure to the existing js files these are the issues I ran in to.

@RemusMar
Copy link
Contributor

RemusMar commented Oct 6, 2020

If I understand correctly the plan is to still include a "/build/three.js" in addition to the "/build/three.module.js".

Yes. However, it's questionable if this approach does make sense.
When examples/js is removed, there are only a few use cases left where three.js and three.min.js are still useful.

@Mugen87 @mrdoob

Michael,
In fact to keep "three.min.js" for at least 2 more years is a MUST.
Not because all my samples are based on it.
But because many thousands of files and Google top dogs are based on it!
Example: https://www.google.com/search?source=hp&q=webgl+benchmark

On the other hand, from my point of view, "three.min.js" means faster development and testing.
Not to mention that it works offline and you don't need localhost.
Just put all the files in a folder somewhere, use Firefox and double click the HTML file.
I always loved that for development!

Ricardo should also think about all of these.
cheers

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 6, 2020

The removal of three.js and three.min.js is something that can be discussed and planned when examples/js is gone. It was just important for me to highlight their lost in significance when you can't import files from examples/js anymore.

@Bug-Reaper
Copy link
Contributor Author

I think the complexity of maintaining this is being under estimated. Unfortunately I don't think it's so simple with the way examples are set up in the project.

I really like the points you go on to bring up. There's absolutely unforeseen complexity in bundling and the example of nested modules is a good one. To your point, I think we can come to sensible decisions about how to handle bundling nested modules when that time comes. I'm not saying a bundler script will be a set it and forget it situation, merely that it will be lower maintenance.

If the time comes where it's too hard to maintain we can always drop it, but I think it's silly to discount trying on account of problems we don't have yet. It will be easiest to implement now while we still have 1 to 1 parity between /examples/jsm and examples/js. We likely won't be massively re-organizing the /example/jsm module hierarchy and I think we can make incremental updates to the bundler when we do. I'm going to go ahead and start working on proof of work for this (with babel because it's already added?) to put my money where my mouth is as they say.

To Mugen's point, this would help keep some relevancy to three.js and three.min.js while we continue to maintain them. It could also help the hundreds of sites that might be looking for an update compatible with their non module based THREE implementation. The refactoring of a THREE project to use modules can be quite extensive even if you know what you're doing.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2020

I can't speak for the other collaborators but I will not change my mind on this topic. I vote to delete examples/js with the December release in 2020 as discussed and committed here #18749.

@RemusMar
Copy link
Contributor

RemusMar commented Oct 7, 2020

I vote to delete examples/js with the December release in 2020 as discussed and committed here #18749.

I don't have any problem with that.
As long as "three.min.js" is available for another couple of years ...

@Bug-Reaper
Copy link
Contributor Author

Thanks for the input Mugen, I did read through that thread but it appears as more of an announcement as opposed to an explanation for the decision. My assumption is that that simplified development is the primary reason for moving in this direction, are there any others?

I think having a transpilation script we can run to generate /examples/js style includes should be an okay compromise here. It should lessen the amount of maintenance/complication required here drastically. I'd even be okay if it was just a command in the package.json you had to run on your own and the files were not generated by default. There are benefits for some developers and others that will need to transpile anyways. I'd rather we all not have create a transpilation/bundle workflow on our own separately when something could be kept in the main repo to better allow us to collaborate. :)

@sciecode
Copy link
Contributor

sciecode commented Oct 7, 2020

I did read through that thread but it appears as more of an announcement as opposed to an explanation for the decision.

Unfortunately we can't always pin all valid arguments to a single thread, either because the realization of a design change slowly progresses on multiple discussions over the years, or simply because multiple threads about the same subject are created over and over ( like this one ). Collaborators try to minimize noise and segmentation, but it's not always possible.

My assumption is that that simplified development is the primary reason for moving in this direction, are there any others?

The biggest one I see is the ability to use and import sub-modules that minimize redundant code and make for re-usable implementations.

For example, most of the loaders need to create some sort of data parsing structure/class, this is because each loader needs to be self-sufficient so that example/js files are re-usable. However, if we entirely remove the non-modular restriction, we would be able to create a single instance of a DataParser class and import that standard implementation on all the loaders, immediately making development easier and also removing redundant code from all the loaders.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2020

Yeah, good point. We already have to make dirty hacks like embedding the Pass class (the base class of all FX passes) into EffectComposer only to ensure legacy code does not break.

@DefinitelyMaybe
Copy link
Contributor

very good points made all round.

getting and keeping folks on-track/up-to-date sounds like (and from my own experience) a tough issue. gonna try put some thought into this.

@looeee
Copy link
Collaborator

looeee commented Oct 8, 2020

In fact to keep "three.min.js" for at least 2 more years is a MUST.

It will always be possible to generate an ES5 build using Babel. The question we'll need to answer when it comes to that is whether the responsibility for this lies with us or with the developer using three.js.

We've already decided that it will be up to the developer to create ES5 versions of the example files, so it probably makes sense to do the same for build files. To my mind, it also makes sense to do this across the whole library in one release rather than spacing it out, but keeping three.min.js around for a bit longer is fine too.

But because many thousands of files and Google top dogs are based on it!
Example: google.com/search?source=hp&q=webgl+benchmark

This is the top site that comes up for me from that search, and they are using R53 so I don't think this change will affect them too badly: https://www.wirple.com/bmark/

As you can see, old versions of three.js still work just fine. After we make the transition to modules, we can direct anyone who wants an ES5 build without using Babel to use the last version before we removed the ES5 files. They can check out the whole repo of that release and use the docs from that version too.

@Bug-Reaper
Copy link
Contributor Author

Bug-Reaper commented Oct 8, 2020

@looeee You touch on some good points. As mentioned above, I agree that it makes sense to deprecate the ES5 three.min.js and three.js at the same time here. Perhaps that should be it's own separate discussion?

Either way I'd like to come to a consensus about including a babel script in the main repo which can be used to generate old school ES5 style /js/example files. This is in no way about whether anyone's responsible for providing this support. There are contributors, like myself, that are going to need this feature. There are benefits for some developers and others that will need to transpile anyways. I'd rather we all not have create a transpilation/bundle workflow on our own separately when something could be kept in the main repo to better allow us to collaborate.

I think it's a fair compromise to allow us one file in the main repo so that we can work on babel ES6 to ES5 transpiler script together. Is there really an issue there? Allowing contributors to work on a feature they need together in the main repo?

I'm not asking collaborators for any help or resources in doing this, I'm simply asking you allow people that need this to be able to work on it together in the main repo. If I make a PR for this and it works, would you really vote to reject it?

@DefinitelyMaybe
Copy link
Contributor

If I make a PR for this and it works

I mean, I'm happy to see this get started

would you really vote to reject it?

tehe all bets will be off if it fails the linting pass 😂

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 8, 2020

Is there really an issue there?

Yes, since the repository should not promote deprecated coding patterns.

@Bug-Reaper
Copy link
Contributor Author

Bug-Reaper commented Oct 8, 2020

Yes, since the repository should not promote deprecated coding patterns.

It's not officially deprecated yet if we're not sacking three.js + three.min.js (acknowledged the consensus ITT is we should sack those too) and having a babel script you have to run manually on your own is hardly a glowing endorsement. I agree we should definitely encourage people to use modules instead and have a warning in the babel script and generated files about it. I disagree allowing contributors to work on a babel script together for people in situations which cannot use modules for whatever reason is promoting a deprecated coding pattern. Mainly because there are still situations where using modules is infeasible/impractical. The docs acknowledge this need. I think we can safely add one file for the people that need it to work on it together.

@looeee
Copy link
Collaborator

looeee commented Oct 9, 2020

I agree that it makes sense to deprecate the ES5 three.min.js and three.js at the same time here.

I meant we should deprecate examples/js, three.min.js, and three.js at the same time, i.e. remove all ES5 code in one release rather than spread over multiple releases.

@RemusMar
Copy link
Contributor

RemusMar commented Oct 9, 2020

@Mugen87

Yes, since the repository should not promote deprecated coding patterns.

You can still run DOS games in Windows 10.
And that doesn't mean Microsoft is promoting "deprecated coding patterns".

@gigablox
Copy link

Just to clarify the example js modules as maintained in this project do not require node, npm, or any build framework to use.

Well let's not forget that building a production ready application means bundling up your code :)

I appreciate bundling tools like Rollup that are available but think we should consider a couple questions:

  • Is it fair to assume if developers want to use THREE in production they also need to use one of these bundling tools?
  • Is it fair to drop support for other libraries that rely on updates to ES5/UMD modules in the examples folder?

My personal feelings about this:

This library is a decade old. There's an enormous ecosystem out there that relies on the modules in the examples folder written in ES5/UMD. I don't think it's fair to drop support for an entire ecosystem.

I think people forget that you can still use ES6 without a module bundling pattern. I use ES6 everyday but do not use module bundling patterns in my frontend applications. I have worked in enterprise shops where build tools become very custom by necessity and would be unable to incorporate a module bundling pattern.

What should we do?

Let's compile the ES6 Modules into ES5/UMD modules for a given distribution after each release.

Yes, since the repository should not promote deprecated coding patterns.

For almost everything in life a solution can still be of great quality using older patterns, techniques and tooling.

As an analogy - In my free time I enjoy carving stone with point chisels. The tools and techniques are different from power tools, but in the end the sculpture will still be of high quality. I've exercised a personal preference to use point chisels because I enjoy using them and have the skills needed to produce something myself or others are happy with.

I feel the same about ES5/UMD modules. I've been able to find patterns, techniques and tooling that uphold really high quality code bases and want to continue exercising that personal preference.

@DefinitelyMaybe
Copy link
Contributor

Let's compile the ES6 Modules into ES5/UMD modules for a given distribution after each release.

I agree with what looeee said.

Is it fair to assume [...]

what? We're talking about what approach we'd prefer, the 'assumption' comes afterwards. The preference seems to be towards encouraging others to use modules but (assuming some folks will still want old THREE) offering a pathway for those that really want it.

@donmccurdy
Copy link
Collaborator

Let's compile the ES6 Modules into ES5/UMD modules for a given distribution after each release.

This can be done by anyone; that cost does not need to be carried by the three.js maintainers. I'd like to reiterate what @gkjohnson said above, the cost of maintaining both examples/js and examples/jsm directories is high. We cannot do this indefinitely, and it's clear that ES6 Modules are the more modern of the two approaches. Consider the following costs:

  • Creating and maintaining the automation
  • Debugging release failures when automation breaks
  • Ensuring that all pull requests update the source file, not the generated one
  • Maintaining documentation that explains how both workflows are used
  • Answering bug reports and support questions from users trying to use both CJS and ES6 workflows

That last item is quite possibly the biggest. As long as two copies of everything are available in this repository, both will be seen as fully supported. We regularly spend time helping users who confuse the two workflows or try to use an ES6 Module loader with the CJS core library, which fails in complicated ways.

We can rephrase the problem simply: all of our examples — which are important but optional parts of the three.js library — currently do not use any module syntax at all. Not UMD, not CommonJS, not ES6 Modules. They simply patch a THREE global namespace. We would like to update that, using ES6 import/export syntax instead, and there have been many early warnings that this change was planned.

There's an enormous ecosystem out there that relies on the modules in the examples folder written in ES5/UMD. I don't think it's fair to drop support for an entire ecosystem.

I don't think it's fair to say that anything in the three.js ecosystem is so dependent on global THREE.* namespaces that it couldn't be updated to use import/export syntax, or to transpile to ES5, or to use a bundler. There are number of workarounds here, and we'd be happy to work with users to help find a suitable option for them.

@gigablox
Copy link

gigablox commented Oct 12, 2020

the cost of maintaining both examples/js and examples/jsm directories is high.

I'd like to dig into this a bit more. I've written a lot of custom tooling and build automation scripts for front end applications and would be happy to help in any way I can.

Creating and maintaining the automation
Debugging release failures when automation breaks

Help me understand the maintenance tax a little more, is this something unique to the THREE codebase? In my experience this type of code is usually the longest lived needing the least amount maintenance. These are scripts you write once and don't look at again for long periods of time.

Ensuring that all pull requests update the source file, not the generated one

Maybe a small script or test could help with this in the release workflow.

Maintaining documentation that explains how both workflows are used

I would also vote to drop the documentation for global namespaces. I think it's silly to support documentation for two workflows. This isn't a bad thing. Most libraries that bundle their code for different contexts, UMD/ES6 modules only have one set of docs.

Answering bug reports and support questions from users trying to use both CJS and ES6 workflows.

I think the volume of issues related to something like this comes relative to the size of THREE's popularity. You and I see these types of issues on Stack Overflow all the time. A user that can't distinguish between the two workflows is likely a new programmer inspired by the library and is just trying to learn the basics of programming in general.

If the goal is to reduce the volume of issues specifically related to confusion between the two workflows then removing the ES5 code would probably help with that - but I doubt the volume of issues overall will change. A new programmer will always be stuck at the next question that may or may not be related to the library.

How to reduce the volume of issues overall?

If the real goal is to reduce the volume of issues overall maybe stricter issue policies can help with that. I see you guys doing a great job with this already using tags like Help (please use the forum) but maybe there needs to be more of these types of things.

More generally it might be best to just descope some types of issues the THREE contributors are willing to discuss and investigate if they are currently feeling overwhelmed by the total volume.

Couple ideas:

  • At time of writing suggestions and enhancements have (271) open issues. These labels seem to generate a lot noise. Maybe only take PR ready / checks passed as the actual suggestion. Insta-close everything else and mark as Discussion (please use the forum).

  • At time of writing loaders have (61) open issues. This label also seems to generate a lot of noise. I see a lot issues with this label related to suggestions and enhancements or poorly formed bug reports. Maybe only take well formed bug reports and PR ready / checks passed for suggestions. Insta-close everything else and mark accordingly.

I don't think it's fair to say that anything in the three.js ecosystem is so dependent on global THREE.* namespaces that it couldn't be updated to use import/export syntax, or to transpile to ES5, or to use a bundler.

I agree anything can be updated but if we can find a way to do a little bit of work to continue supporting these users in a sustainable way I agree with @Bug-Reaper in saying:

I'd rather we all not have create a transpilation/bundle workflow on our own separately when something could be kept in the main repo to better allow us to collaborate.

We would be collectively saving these users an enormous amount of time from upgrading their applications/libraries, build systems and documentation.

@gkjohnson
Copy link
Collaborator

gkjohnson commented Oct 12, 2020

And the thing actually worked... 😐

FWIW this only seems to actually work in a small amount of cases. When it doesn't work it fails in extremely confusing ways and we've had several issues filed related to that granted it happens with build processes as well. Arguably if you're concerned about the newbie giving them multiple ways to use the same files is more error prone and confusing.

Maybe tangential but it might be worth informing people that they have two copies of three.js included in the page via a warning in the console (even if they're same version) which can cause issues unless care is taken to not mix them. I believe React does this for similar reasons React may actually just point to this as a possible source of an error. That could least help move people away from mixing these modalities when learning.

I reached the same conclusion as @Bug-Reaper. Today I'm having a look at creating a script that builds examples/js out of examples/jsm files.

If this is the new plan I'd be happy to help revive #15526 / #15543 (which have now been deleted from the project) which builds every module file to an ES6 one. Given that some examples are spread among so many files (Shader Nodes, for example) and we may be interested in splitting some of the modules up into multiple files it's probably worth upgrading the rollup script to take an explicit list of files we want to convert and output. We should be able to automatically create dependencies between the files that are output, as well.

@DefinitelyMaybe
Copy link
Contributor

DefinitelyMaybe commented Oct 13, 2020

One big thing we lose when not copying files is being able to edit them

I agree although if we can get to classes all over I'd hope something like:

import orbitalcontrols from  orbitalcontrolsURL

class mycontrols extends orbitalcontrols {
// do the edits I care about
}

and then later

let controls = new myorbitalcontrols

@devingfx
Copy link
Contributor

devingfx commented Oct 14, 2020

One big thing we lose when not copying files is being able to edit them

I agree although if we can get to classes all over I'd hope something like:

import orbitalcontrols from orbitalcontrolsURL

class mycontrols extend orbitalcontrols {
// do the edits I care about
}

and then later

let controls = new myorbitalcontrols

You can do that already... even if the parent "class" is simple js function !

Code actually working (in a debugger quick test) :

Promise.all([
    import('https://unpkg.com/three/build/three.module.js')
        .then( mod=> [mod.Camera, mod.WebGLRenderer] ),
    import('https://unpkg.com/three/examples/jsm/controls/OrbitControls.js')
        .then( mod=> mod.OrbitControls )
])
.then( ([
    [ Camera, WebGLRenderer ],
    OrbitControls
])=> new ( class extends OrbitControls {} )( new Camera, (new WebGLRenderer).domElement )
)
.then( console.log )

... or simpler syntax :

(async function() {

let { Camera, WebGLRenderer } = await import('https://unpkg.com/three/build/three.module.js')
,	{ OrbitControls } = await import('https://unpkg.com/three/examples/jsm/controls/OrbitControls.js')

class Con extends OrbitControls { }

let my = new Con( new Camera, (new WebGLRenderer).domElement )
console.log( my )

})()

@DefinitelyMaybe
Copy link
Contributor

aside from that aynom function and worrying about async/await promises, cool

@looeee
Copy link
Collaborator

looeee commented Oct 15, 2020

class mycontrols extend orbitalcontrols {
 // do the edits I care about
 }

Ideally, this is the pattern we should be promoting, rather than telling users to edit the original files when making changes. However, the examples are not written with extensibility in mind so there are strong limits on what you can achieve. In my experience, you end up having to copy the entire original example into the extended class's constructor to get it to work so there's no point in using extend.

For example, the most common requested change for OrbitControls is to limit the pan. This is easily accomplished as demonstrated in @Mugen87's fiddle from that thread.

In short, you add minPan and maxPan vectors and clamp controls.target in the controls.update method.

I had a go at doing this by extending OrbitControls. You can create an extended class, and it's works fine. However, problems become apparent when you start to make changes. You can't simply extend the update method:

class OrbitControlsPanLimit extends OrbitControls {
    constructor(object, domElement) {
        super(object, domElement);
    }
  
    update() {
        super.update();
        console.log('Custom update function');
    }
}

This extended class works (glitch), but this new OrbitControlsPanLimit.update method is ignored. The original OrbitControls.update method is still used.

You can overwrite it by redefining it in the constructor:

class OrbitControlsPanLimit extends OrbitControls {
	constructor(object, domElement) {
		super(object, domElement);

		this.update = () => {
			console.log('Custom update function');
		}
	}
}

You can't use super.update() here so the only option is to copy the entire original update method over. However, that method relies on lots of this stuff from within OrbitControls, which is shared between all the methods.

	//
	// internals
	//

	var scope = this;

	var changeEvent = { type: 'change' };
	var startEvent = { type: 'start' };
	var endEvent = { type: 'end' };

	var STATE = {
		NONE: - 1,
		ROTATE: 0,
		DOLLY: 1,
		PAN: 2,
		TOUCH_ROTATE: 3,
		TOUCH_PAN: 4,
		TOUCH_DOLLY_PAN: 5,
		TOUCH_DOLLY_ROTATE: 6
	};

	var state = STATE.NONE;

	var EPS = 0.000001;

	// current position in spherical coordinates
	var spherical = new THREE.Spherical();
	var sphericalDelta = new THREE.Spherical();

	var scale = 1;
	var panOffset = new THREE.Vector3();
	var zoomChanged = false;

	var rotateStart = new THREE.Vector2();
	var rotateEnd = new THREE.Vector2();
	var rotateDelta = new THREE.Vector2();

	var panStart = new THREE.Vector2();
	var panEnd = new THREE.Vector2();
	var panDelta = new THREE.Vector2();

	var dollyStart = new THREE.Vector2();
	var dollyEnd = new THREE.Vector2();
	var dollyDelta = new THREE.Vector2();

The end result is that you'll have to copy the nearly entire original OrbitControls into the OrbitControlsPanLimit constructor which defeats the purpose of extending the class. Unless we write the controls as a class with extensibility in mind, I don't think extending it is viable.

@DefinitelyMaybe
Copy link
Contributor

thank you @looeee for chiming in there. I was thinking maybe I'd missed an easy solution within my own endeavors but now that you mention it, that's pretty much where I got up to myself.

@gigablox
Copy link

gigablox commented Oct 15, 2020

Ideally, this is the pattern we should be promoting, rather than telling users to edit the original files when making changes.

Careful, that's closely treading towards an inheritance vs composition argument.

Ideally a library shouldn't be promoting any patterns. It should be promoting it's features and how it aims to solve your problems.

It also shouldn't be assuming a developers workflow, stack, build system, use case. A great library is as accommodating to the many complex needs of it's community as possible.

What's new today is old tomorrow, patterns come and go. The only constant then would be software that offers great support for the many use cases along the way to maintain as much backwards compatibility as possible.

You can still run DOS games in Windows 10.

@DefinitelyMaybe
Copy link
Contributor

DefinitelyMaybe commented Oct 16, 2020

inheritance vs composition argument

please no. the solution to this 'argument' is to use the best tool for the job. there is a place for inheritance, composition, functional, test-driven... you name it.

Since we're talking about how other developers (use, reuse, modify) three.js, it is valid to promote a pattern which would be readily understood and useable without stepping outside js browser features.

promoting does not mean that one cannot use a different style.

as much backwards compatibility as possible

yes and no.

It should be promoting it's features and how it aims to solve your problems

perhaps so that we're clear, what is the problem/feature set for you?

@DefinitelyMaybe
Copy link
Contributor

DefinitelyMaybe commented Oct 16, 2020

It also shouldn't be assuming a developers workflow, stack, build system, use case

I mostly agree. threejs use case is currently the browser. the caveat there is some of our loaders are useful for some node applications from what I've heard.

The only constant then would be software that offers great support for the many use cases along the way

change is the only constant. developers use the tool that they like and sometimes we give other things a go.

as an aside:

It should be promoting it's features and how it aims to solve your problems

which came first? the feature, the pattern or the problem?
surely the pattern helped solve the problem and then became a feature
...or was it the feature that created the problem and we found a pattern to solve it?

@RemusMar
Copy link
Contributor

which came first? the feature, the pattern or the problem?

Which came first? The Hen or the Egg?
Some people say the Rooster ...

@Bug-Reaper
Copy link
Contributor Author

Bug-Reaper commented Oct 17, 2020

Great discussion all around, thanks everyone for all the input.

I'd be curios to what you guys think about which bundler ( rollup, babel, parcel, webpack, etc ) is best suited for the task of transpiling our ES6 example modules. I believe @gigablox mentioned having experience here and I'm sure others do as well.

The current repo already contains babel, rollup and a couple related plugins. I went ahead and started hacking away at this tonight and I have an extremely rough rollup config script to share:

// jsm-transpiler.js
export default [
  {
    input: './examples/jsm/controls/OrbitControls.js',
    output: {
      banner:"//warning this file was generated automatically",
      file: './examples/js/controls/OrbitControls.js',
      name:'OC',
      footer:'THREE["OrbitControls"]=OC.OrbitControls',
      format: 'umd'
    }
  }
];

This rollup config script does indeed convert the OrbitControls module into a non-module .js file include which assigns THREE.OribitControls the appropriate constructor. It worked, which is cool :) ! It also bundled the 40k lines of THREE.js into the output file, not so cool haha. I'm also lazily polluting the global variable space by declaring an intermediary global var called OC to help transport the OrbitControls constructor onto THREE.

Rollup seems to have some really cool features I think can address a lot of our problems. Notably mapping and other controls for making sure the correct nested modules get included/excluded. The ability to inject code before and after the transpiled payload via header/footer/intro/output properties.

I'm cautiously optimistic we can accomplish what we need with a tricked out rollup config script. But it'd be great if someone who's researched/understands the differences between the many bundlers could weigh in here though. We'll need something fairly robust to handle modules as they become more awesome and I'd bet some transpile code better than others.

@gigablox
Copy link

gigablox commented Oct 18, 2020

Here's my take on it:
#20529

This is a poc custom build script that converts all JSM modules into JS global namespaced modules in about 30 seconds. Had pretty good success with this method. Needs more testing but tried a few of the more complex modules like the GLTFLoader in a hello world and it was fine.

Could use help from any seasoned RegExp wizards :) to wrangle some edge cases you can read more about in the PR.

@mrdoob
Copy link
Owner

mrdoob commented Dec 18, 2020

Chrome's Import Maps Intent to Ship:
https://groups.google.com/a/chromium.org/g/blink-dev/c/rVX_dJAJ-eI

@mrdoob
Copy link
Owner

mrdoob commented Jan 13, 2021

With import maps it would look like this:

<script type="importmap">
{
  "imports": {
    "three": "js/libs/three.module.js",
    "OBJLoader": "js/libs/three/OBJLoader.js",
    "OrbitControls": "js/libs/three/OrbitControls.js"
  }
}
</script>
<script type="module">
    import * as THREE from 'three';
    import { OBJLoader } from 'OBJLoader';
    import { OrbitControls } from 'OrbitControls';

    console.log( THREE, OBJLoader, OrbitControls );
</script>

Not as pretty as it was before, but takes care of import dependencies/order for you and doesn't require a bundler.

Actually, it simpler than that:

<script type="importmap">
{
  "imports": {
    "three": "./js/three.module.js"
  }
}
</script>
<script type="module">
    import * as THREE from 'three';
    import { OBJLoader } from './js/OBJLoader.js';
    import { OrbitControls } from './js/OrbitControls.js';

    console.log( THREE, OBJLoader, OrbitControls );
</script>

The only bare specifier we need is three. OBJLoader and OrbitControls do not need to be added to importmap list.

@gkjohnson
Copy link
Collaborator

<script type="importmap">
{
 "imports": {
   "three": "../js/three.module.js"
 }
}
</script>

The only bare specifier we need is three. OBJLoader and OrbitControls do not need to be added to importmap list.

Another option is to use the trailing slash notion in the import map to specify a root that's used when importing something like "three/examples/jsm/OrbitControls.js". This can be useful if users are loading their files from a CDN, trying to reduce noise in the import statements, or building a third party package with three.js dependencies:

{
  "imports": {
    "three": "http://unpkg.com/three/build/three.module.js",
    "three/": "http://unpkg.com/three/",
  }
}

This is all you'd need to do in order to import any file from three.js examples with import maps. See more information on the spec here.

Chrome's Import Maps Intent to Ship:

Have Safari or Mozilla commented on support? Chrome has rushed features out before that have wound up being significantly changed or deprecated (web components, HTML imports) so this still feels like it's a ways from being a real solution.

@mrdoob
Copy link
Owner

mrdoob commented Jan 14, 2021

Another option is to use the trailing slash notion in the import map to specify a root that's used when importing something like "three/examples/jsm/OrbitControls.js"

Nice! Yeah... That works for jsfiddles, glitch, etc
In my example I'm focusing on a nice pattern to use in the examples in this repo.

Have Safari or Mozilla commented on support? Chrome has rushed features out before that have wound up being significantly changed or deprecated (web components, HTML imports) so this still feels like it's a ways from being a real solution.

Yeah, well aware... But I think that without importmaps modules are half-backed, so I'm kind of confident that they'll end up adopting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests