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

Importing examples jsm modules causes bundlers to bundle three.js source code twice #17482

Closed
adrian-delgado opened this issue Sep 12, 2019 · 79 comments
Labels

Comments

@adrian-delgado
Copy link

Importing from three/examples/jsm/.../<module> causes bundlers (tested with rollup) to include the library twice (or multiple times).

For example, when doing import { OrbitControls } from 'three/examples/jsm/controls/OrbitControls', the bundler will follow the import and in the OrbitControls.js the imports come from ../../../build/three.module.js. However, there is no way for the (external) bundler to know that ../../../build/three.module.js is the same module as three.

A solution for this would be to treat the examples modules as external packages and import from three instead of ../../../build/three.module.js. This might break the rollup config of three.js, but it should be possible to tell rollup that three is an alias for the main entry point of three (src/Three.js).

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 12, 2019

(tested with rollup)

I can not confirm this with rollup. If you are doing it like in the following project setup, everything works as expected.

https://github.com/Mugen87/three-jsm

@adrian-delgado
Copy link
Author

If you treat three as external dependency:

export default {
	input: 'src/main.js',
	external: ['three'],
	output: [
		{
			format: 'umd',
			name: 'LIB',
			file: 'build/main.js'
		}
	],
	plugins: [ resolve() ]
};

then the output should not contain the source code of three.js, yet it includes everything.

If, however, you don't import the OrbitControls, then the output will only include the source code of the main.js file.

you can try it out by commenting out the OrbitControls import, and then building again (but with 'three' as external dependency).

@gkjohnson
Copy link
Collaborator

gkjohnson commented Sep 12, 2019

This is related to #17220 -- one of the solutions proposed there was to replace the main field in package.json with the module build path but that would not fix this use case.

Just to be clear the issue here is that while three is being marked as external to build a separate package that is dependent on three in the rollup config that does not catch the hard reference to ../../../build/three.module.js and includes it in the build. For example building the following file will inadvertently include the OrbitControls code and the threejs code in the bundle, as well as import another copy of three when built with @adrian-delgado's posted config.

// src/main.js
import * as THREE from 'three';
import { OrbitControls } from 'three/examples/jsm/controls/OrbitControls.js';

console.log(THREE, OrbitControls);

@adrian-delgado it might be worth noting that even if the path in OrbitControls.js is changed to three OrbitControls will still be included in your bundle which may or not be desired and could result in at least the OrbitControls code being included twice in dependent applications.

I don't mean to propose this as a long term or best solution but changing the config to mark OrbitControls (and all files in the three folder) as external would solve this in both cases:

export default {
	// ...

	external: p => /^three/.test(p),

	// ...
};

@adrian-delgado
Copy link
Author

I don't mean to propose this as a long term or best solution but changing the config to mark OrbitControls (and all files in the three folder) as external would solve this in both cases:

For some reason I expected rollup to treat 'three/examples/jsm/controls/OrbitControls.js' as external too by default. So your proposed solution is good for my use case.

The related #17220 is very relevant. The conversation should probably continue there.

@mrdoob
Copy link
Owner

mrdoob commented Sep 14, 2019

So what happens if you do this?

// src/main.js
import * as THREE from 'three/build/three.module.js';
import { OrbitControls } from 'three/examples/jsm/controls/OrbitControls.js';

console.log(THREE, OrbitControls);

@drcmda
Copy link
Contributor

drcmda commented Sep 14, 2019

It would work, but it's not feasible, because any other lib or piece of code that's dependent on three will import from "three" and then it breaks again. Package.json normally tells the environment how to resolve, "build/three.module" is a distribution detail that shouldn't leak out. When resolution is skipped that just invites namespace problems.

@EliasHasle
Copy link
Contributor

	external: p => /^three/.test(p),

@gkjohnson What if the user wants to include the "three" instance and OrbitControls in the bundle?

@greggman
Copy link
Contributor

greggman commented Sep 15, 2019

Not sure it's related a similar happens if you try to use modules live like this

import * as three from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/three.module.js';
import { OrbitControls } from 'https://threejs.org/examples/jsm/controls/OrbitControls.js';

loads three.js twice, once from the CDN and again from threejs.org

Maybe that's not the way modules are supposed to be used with three but just going from pre 106 there are 1000s of sites and examples that do

<script src="https://cdnjs.cloudflare.com/ajax/libs/three.js/105/three.min.js"></script>
<script src="https://threejs.org/examples/js/controls/OrbitControls.js"></script>

All the examples show using modules live instead of building(bundling) so in a sense they aren't showing the actual way to use three.js like they used to. In other words, the old examples worked out of the box. The new examples don't AFAIK. In order for an example to work you'd need to extract the JavaScript out of the example and put in a separate .js file, then put three.js locally (probably via npm). Fix all the paths in the examples so they are package based paths (no ../.././build), and finally use rollup

That's pretty big change from the non module versions for which just changing the paths was enough

@gkjohnson
Copy link
Collaborator

@mrdoob

With @adrian-delgado's original config three.js will be included once and orbit controls will be included once and no packages will be marked as external. With the config I proposed there will be an external dependency on three/build/three.module.js and three/examples/jsm/controls/OrbitControls.js in the produced bundle.

@EliasHasle

What if the user wants to include the "three" instance and OrbitControls in the bundle?

Then the external field should be excluded in which case a single copy of three and orbit controls will be included in the bundle. rollup-plugin-node-resolve (which is needed for rollup to support module resolution and is being used in the above configs) defaults to use the module field of package.json (see the mainFields option) so the orbit controls three reference and "three" will resolve to the same script. If mainFields is changed to ["main", "module"] so "main" is used instead of "module" in package.json then two copies of three will be included here and things will break in the ways that have been mentioned previously. However it does require changing that field. If "main" is used, though, then rollup-plugin-commonjs will likely need to be needed, as well, because rollup does not know how to process commonjs files that use require by default.

@greggman

Unfortunately I don't think a naive replacement of modules will work so easily in this case. None of the proposed solutions will address this case and I don't think there's anything official at the moment that can be used to help the case of importing the core script and example from separate hosts. Import maps are the only thing that's in the works to help with this as far as I know. If both the example and three are imported from the same host then only a single copy of three will be loaded:

import * as three from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/three.module.js';
import { OrbitControls } from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/examples/jsm/controls/OrbitControls.js';

// or

import * as three from 'https://threejs.org/build/three.module.js';
import { OrbitControls } from 'https://threejs.org/examples/jsm/controls/OrbitControls.js';

Depending on the use case maybe it's preferable to continue using the classic script tags?

@mrdoob
Copy link
Owner

mrdoob commented Sep 16, 2019

@greggman

Not sure it's related a similar happens if you try to use modules live like this

import * as three from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/three.module.js';
import { OrbitControls } from 'https://threejs.org/examples/jsm/controls/OrbitControls.js';

Yeah... Don't use modules like that 😁

@greggman
Copy link
Contributor

greggman commented Sep 17, 2019

Yeah... Don't use modules like that 😁

Agreed. It's just arguably the docs and examples are targeting mostly inexperienced developers and the fact that jsm examples are the default and none of them will work without a builder nor will they work via any CDN is a kind of big change.

It used to be you could basically view source on an example, copy and paste into jsfiddle/codepen etc, fix the paths in the script tags and it would run. Now though all the examples won't run unless you link directly into the three.js site and watch them break each time the version gets bumped. (yes I know the non module examples exist but those are not the ones linked to from https://threejs.org/examples)

@gkjohnson

import * as three from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/three.module.js';
import { OrbitControls } from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/examples/jsm/controls/OrbitControls.js';

Doesn't work, OrbitControls are not on the CDN and the paths inside the OrbitContrls ../../../bulild/three.js is not the correct path to make it work

// or

import * as three from 'https://threejs.org/build/three.module.js';
import { OrbitControls } from 'https://threejs.org/examples/jsm/controls/OrbitControls.js'

Also doesn't work as it will break every time three.js pushes a new version

Maybe push the examples/js folder to a CDN and three such that just fixing the urls in the example code will still work? That means three.module.js needs to be at

https://cdnjs.cloudflare.com/ajax/libs/three.js/108/build/three.module.js

build added to the path

@donmccurdy
Copy link
Collaborator

To mention the options, other CDNs like jsdelivr or unpkg do support ES modules:

@donmccurdy
Copy link
Collaborator

It used to be you could basically view source on an example, copy and paste into jsfiddle/codepen etc, fix the paths in the script tags and it would run...

I think we'll need import maps to do anything useful about that, for better or worse.

Now though all the examples won't run unless you link directly into the three.js site

I would really not encourage anyone to link directly to live scripts on the threejs site... that will never be a good idea. There are versioned alternatives, per comment above.

The documentation that would, ideally, answer these questions is the Import via modules page. Are there cases we should cover there? I suppose mentioning the CDNs would be a good idea.

@greggman
Copy link
Contributor

Mentioning the CDNs would be a good idea. Also mentioning that the cloudflare CDN, the first hit, on Google is no good for modules (unless that changes)

@mrdoob
Copy link
Owner

mrdoob commented Sep 24, 2019

@greggman

It used to be you could basically view source on an example, copy and paste into jsfiddle/codepen etc, fix the paths in the script tags and it would run.

I'm on your side. The worst part of modules is that you can't access camera or renderer from the console in the examples anymore 😟

@mrdoob
Copy link
Owner

mrdoob commented Sep 24, 2019

How about we start using unpkg?

@donmccurdy
Copy link
Collaborator

How about we start using unpkg?

Do you mean start using it in documentation like the Import via modules page, or using it in the project somehow?

@donmccurdy
Copy link
Collaborator

The worst part of modules is that you can't access camera or renderer from the console in the examples anymore

Yeah, that's frustrating. I've been throwing this (or similar) into the examples when developing locally:

Object.assign( window, { camera, renderer, scene } );

I assume that's something we hope to solve with a dev tools extension?

@donmccurdy
Copy link
Collaborator

One idea that would take some investigation, but could be interesting... if we'd be willing to add an import map polyfill to all the examples, I think we could make the imports used there 100% copy/paste compatible with npm- and bundler-based workflows. For example:

<script defer src="es-module-shims.js"></script>
<script type="importmap-shim" src="importmap.dev.js"></script>

<!-- ... -->

<script type="module-shim">
  import { Scene, WebGLRenderer } from 'three';
  import { GLTFLoader } from 'three/examples/jsm/loaders/GLTFLoader.js';
  
  // ...
</script>

@mrdoob
Copy link
Owner

mrdoob commented Sep 24, 2019

How about we start using unpkg?

Do you mean start using it in documentation like the Import via modules page, or using it in the project somehow?

Instead of pointing to https://threejs.org/build/. Currently we're using that link in ISSUE_TEMPLATE.

And @greggman could probably switch from https://cdnjs.cloudflare.com/ajax/libs/three.js/108/ to https://unpkg.com/[email protected]/?

Just seems like unpkg solves the problems we're discussing here.

@mrdoob
Copy link
Owner

mrdoob commented Sep 24, 2019

Yeah, that's frustrating. I've been throwing this (or similar) into the examples when developing locally:

Object.assign( window, { camera, renderer, scene } );

Ugh! Haha

I assume that's something we hope to solve with a dev tools extension?

Yes! 🤞

@mrdoob
Copy link
Owner

mrdoob commented Oct 7, 2019

@greggman

Not sure it's related a similar happens if you try to use modules live like this

import * as three from 'https://cdnjs.cloudflare.com/ajax/libs/three.js/108/three.module.js';
import { OrbitControls } from 'https://threejs.org/examples/jsm/controls/OrbitControls.js';

Yeah... Don't use modules like that 😁

So today I found myself doing just that... 😅 It's a bad habit indeed, but the problem is that most things kind of work but if something breaks is pretty hard to nail down.

In my case I was importing three.module.js from dev and OBJLoader from master. OBJLoader imported three.module.js from master so the BufferGeometry didn't have the new usage property, and WebGLRenderer did not render the mesh because it didn't find usage, everything else worked though 😶

This is pretty hairy...

@greggman
Copy link
Contributor

greggman commented Oct 7, 2019

I think it's just something to get used to. Now that I think I get it I'm fine with the way it is.

BTW I updated threejsfundamentals to all be esm based so 🤞

@greggman
Copy link
Contributor

greggman commented Oct 7, 2019

It does kind of seem like it might be good to have a three.module.min.js though (or is that three.min.module.js 😜)

@Rumyra
Copy link

Rumyra commented Nov 12, 2019

+1

I am just importing three & orbit controls as ES6 modules & because (it appears) orbit controls refers to three within the build folder it took me a while to figure out my paths

Super fan we can use three as modules, but would be nice to have more flexibility around this, I'm not going to go into the orbit controls file and start messing about, assuming this is the case with other modules too.

Also +1 for a three.min.module.js 😎

@0b5vr
Copy link
Collaborator

0b5vr commented Dec 24, 2019

moving from #18239, I got caught in it a similar problem by doing npm link on another package that uses three.js.

@yushijinhun
Copy link
Contributor

I've developed a plugin three-minifier which may help solve this problem.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Apr 14, 2021

How about having a separate npm package for examples only for end users that has three js as a dependency?

Or to riff on this idea a bit, maybe a pre-publish script that generates a new directory within the existing npm package? It's only the examples/jsm/* code that needs to be transformed here. In that case we'd have something like:

  • examples/js/*: Legacy scripts depending on global THREE.*. For copy/paste workflows.
  • examples/jsm/*: ES modules with relative imports from ../../build/three.module.js. For older CDNs and development in three.js repository.
  • examples/npm/*: ES modules with bare imports from three. For bundlers, modern CDNs, and Import Maps.

The latter could be generated by a pre-publish script and not checked into the repository.

Could also (needs more research?) set up an exports map so that bundlers automatically resolve a shorthand path, like:

import { OrbitControls } from 'three/controls/OrbitControls.js';

It could also have cdn variants that assumes three js has been already imported..

That's essentially what examples/js/* provides, but it isn't compatible with ES modules. I don't think we want to encourage a workflow that mixes global variables with ES modules.

@mrdoob
Copy link
Owner

mrdoob commented Apr 14, 2021

@gkjohnson @donmccurdy

I think I'm all for adopting https://skypack.dev/ and having a script that converts ".../build/three.module.js" to "three" at npm publish time 👍

@mrdoob
Copy link
Owner

mrdoob commented Apr 14, 2021

Just to clarify... The examples would continue using ".../build/three.module.js" but because in npm we'd be using "three" people using jsfiddle, codepen, glitch, ... would have to use https://skypack.dev/ instead of https://unpkg.com/.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 14, 2021

I think that's a good compromise (considering that we can finally solve this issue^^).

@donmccurdy
Copy link
Collaborator

Just to confirm, we'd be transforming the examples/jsm folder at publish time — not creating a 3rd directory — such that it's compatible with skypack and modern bundlers, with the understanding that unpkg and older CDNs won't work for examples/jsm in future releases?

Could we do an "alpha" release with these changes first, to confirm everything works as expected on skypack? Would be unfortunate to drop unpkg support and then find that skypack is not working as intended, although I don't expect that.

(/cc @drcmda since you'd brought up skypack before)

@gkjohnson
Copy link
Collaborator

gkjohnson commented Apr 15, 2021

Just made PR #21654 with a prepublish script.

@donmccurdy

just to confirm, we'd be transforming the examples/jsm folder at publish time — not creating a 3rd directory

I would be concerned that publishing a third folder would cause confusion with some people pulling from the wrong folder etc.

with the understanding that unpkg and older CDNs won't work for examples/jsm in future releases?

Which other CDNs will break? Only CDNs that pull directly from NPM will break. Are there others? I would think that most CDNs that pull from NPM should be bare-module aware considering that's the predominant way to distribute packages there. I like unpkg but I expect it to break in a lot of cases like the example I posted above and I think it's a problem that it doesn't support bare modules more robustly.

Could we do an "alpha" release with these changes first, to confirm everything works as expected on skypack?

What are you thinking might break? I'm not against a test run of some kind but I'm also thinking that if it doesn't work for some reason a point release could be made with reverted paths.

Could also (needs more research?) set up an exports map so that bundlers automatically resolve a shorthand path, like:

I like this idea. It looks like we could just add this to package.json:

"exports": {
    "./": "./examples/jsm/"
}

@donmccurdy
Copy link
Collaborator

donmccurdy commented Apr 15, 2021

Which other CDNs will break? Only CDNs that pull directly from NPM will break. Are there others? I would think that most CDNs that pull from NPM should be bare-module aware considering that's the predominant way to distribute packages there.

I think https://www.skypack.dev/ is one of a kind at this point. Comparing CDNs I'm familiar with:

  • Unpkg: Static file server.
  • Jsdelivr: Static file server.
  • cdnjs / Cloudflare: Manually curated, doesn't host examples/jsm.
  • Githack: Serves from GitHub; no change.
  • bundle.run: ??? seems to be down.

What are you thinking might break?

I'm not sure what will happen with UMD packages, e.g. dependencies in examples/js/libs/*, like draco_decoder.js. Maybe you just have to get these from another CDN? Maybe they're left untouched by skypack?

Similarly, I don't know what chevrotain.module.min.js is, it seems to be mixing and matching UMD and ES Module syntax.

I like this idea. It looks like we could just add [pkg.exports] to package.json:

I think I'd read somewhere that once you define pkg.exports, all unlisted entrypoints are blocked (at least by Node.js?) so that is the "needs more research" concern. 🤔

@gkjohnson
Copy link
Collaborator

Jsdelivr: Static file server.

Oh I actually didn't realize jsdelivr could pull from npm. It looks like it can pull from Github, as well, so there's a relatively easy work around in this case.

I'm not sure what will happen with UMD packages, e.g. dependencies in examples/js/libs/*, like draco_decoder.js. Maybe you just have to get these from another CDN? Maybe they're left untouched by skypack?

Similarly, I don't know what chevrotain.module.min.js is, it seems to be mixing and matching UMD and ES Module syntax.

Just looking at the chevrotain module on skypack it looks good. It still just sets chevrotain on window and exports it. These cases could be tested in their current form, though, by modifying an example that imports it to point to skypack in a local repo.

It does look like draco_decoder.js might be a problem, though. It looks like skypack tries to convert the require statements to use import syntax but chokes on the "path" and "fs" requires used conditionally in the middle of the emscripten-converted code and will throw an error when it tries to import them. Not exactly sure what to do about this case -- it's a bit of a weird one.

I think I'd read somewhere that once you define pkg.exports, all unlisted entrypoints are blocked (at least by Node.js?) so that is the "needs more research" concern. 🤔

Ha okay that's a good point 😁

@donmccurdy
Copy link
Collaborator

Not exactly sure what to do about this case -- it's a bit of a weird one.

Ok, thanks for checking! I don't think it should prevent us from going ahead with this plan (emscripten has lots of compile options for these situations) but does mean a bit more to test.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 20, 2021

Should be fixed with r128! 🎉

@Mugen87 Mugen87 closed this as completed Apr 20, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 20, 2021

Just to clarify, the current solution doesn't yet work with https://unpkg.com/ (needs ?module).
People should use https://www.skypack.dev/ instead.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 20, 2021

I've added a note in the migration guide for this. Feel free to add additional information if something is missing:

https://github.com/mrdoob/three.js/wiki/Migration-Guide#127--128

@mrdoob
Copy link
Owner

mrdoob commented Apr 20, 2021

Thanks! Did a bit of clean up 👍

@mmncit
Copy link

mmncit commented Apr 28, 2021

Should be fixed with r128! 🎉

Updated the version of "three": "^0.128.0" and "@types/three": "^0.127.1" in package.json. But, still observing the **WARNING: Multiple instances of Three.js being imported**. while running the test.

Question: Does any jest configuration (here) need to be taken care of?

@alexravenna
Copy link

Should be fixed with r128! 🎉

Updated the version of "three": "^0.128.0" and "@types/three": "^0.127.1" in package.json. But, still observing the **WARNING: Multiple instances of Three.js being imported**. while running the test.

Question: Does any jest configuration (here) need to be taken care of?

I have the same issue!

@marcofugaro
Copy link
Contributor

Please share a minimal reproducible example.

@taseenb
Copy link
Contributor

taseenb commented Oct 10, 2021

For those using Webpack, this solution still works (on both Webpack 4 and 5) after the r128 fix: #17482 (comment)

Basically it won't include three in your build (so you can use a CDN, for example) but will include the examples. It works because the imports in the examples are now "three/*" but we only ignore "three". The part related to three.module.js can probably be removed now.

@jashson
Copy link

jashson commented Feb 28, 2022

The issue came back with r137 using webpack (at least for me). With versions lower then r137 i do not get any warnings of multiple instances of three.js. So the solution is currently to stick to r136. I tried r138 which has the same issue like r137

@marcofugaro
Copy link
Contributor

marcofugaro commented Feb 28, 2022

@jashson could you share your configuration/import? does not happen for me

@jashson
Copy link

jashson commented Feb 28, 2022

@marcofugaro sure - i use webpack v5.69.1 and webpack-dev-server v4.7.4. with TypeScript. Three.js is available via npm / package.json. (no CDN)

Here is the essential part of my webpack.config.js

module.exports = {
  entry: {
    js: {import: 'index.ts', filename: 'app.js'},
  },
  ...
}

And here the import of Three.js and OrbitControls:

import {
    PerspectiveCamera,
    Scene,
    WebGLRenderer
} from "three";
import {OrbitControls} from "three/examples/jsm/controls/OrbitControls";

@marcofugaro
Copy link
Contributor

@jashson did a minimal repro with the code you provided and the issue does not happen for me:

webpack-three-test.zip

It's a problem with your personal configuration, the place for this issue is over at stackoverflow.

@jashson
Copy link

jashson commented Feb 28, 2022

@marcofugaro Thanks you for your help and the minimal repro (which works just fine). I'll will check my configuration.

@Zundrium
Copy link

Zundrium commented Mar 8, 2022

@jashson I'm facing the same issue, would appreciate if you could share your solution :)

@jashson
Copy link

jashson commented Mar 8, 2022

@Zundrium I'm currently in a kind of production-hell, so that I had no time to look for the reason in my configuration. My quick solution is to use r136, which runs without warnings.

@Zundrium
Copy link

Zundrium commented Mar 8, 2022

@jashson Alright, thanks for the quick response!

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

No branches or pull requests