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

Docs: Update import instructions in readme. #21113

Closed
wants to merge 1 commit into from

Conversation

donmccurdy
Copy link
Collaborator

The import instructions from README.md are shown on the npm package homepage (https://www.npmjs.com/package/three) and may create confusion about how to import the library. I think it's best to advise imports from the three namespace — any workflow involving copying files into project folders will be messy, running into issues like duplicate imports.

@mrdoob
Copy link
Owner

mrdoob commented Jan 21, 2021

I am not ready to concede defeat on this fight 😅

Here's my proposal: #21128

@DefinitelyMaybe
Copy link
Contributor

DefinitelyMaybe commented Jan 21, 2021

clears throat
YOU WILL NEVER DEFEAT ME!

keep up the good work @mrdoob and co. ya'll are doing super.

@donmccurdy perhaps splitting/sectioning the readme? a browser section followed by a node.js section?
related comment.

@mrdoob
Copy link
Owner

mrdoob commented Jan 21, 2021

Giving #21128 a try. I'll reconsider this one if I see that things don't improve...

@mrdoob mrdoob closed this Jan 21, 2021
@donmccurdy
Copy link
Collaborator Author

I do like #21128, but my remaining concern is that if you start from this line...

import * as THREE from './js/three.module.js';

... and put three.module.js into a js/ folder, there's no longer a way to import anything from examples/jsm/*, since all those files use a relative reference to ../../build/three.module.js. If we'd prefer to not use the npm package name for the import, what about something like:

import * as THREE from './$PATH_TO_THREE/build/three.module.js';

Then at least (1) relative imports from examples/jsm/* will still work, and (2) it's more obvious that the path to the three.js library is whatever you choose.

@donmccurdy donmccurdy deleted the donmccurdy-npm-install branch January 23, 2021 02:10
@mrdoob
Copy link
Owner

mrdoob commented Jan 26, 2021

Yes, I understand.

The problem is, if we do import * as THREE from 'three'; we "force" people to use build systems. Interestingly, https://threejs-journey.xyz/ teaches using <script src="js/three.min.js"></script>, probably because it's more approachable to newcomers.

So... the way I see it, we have 2 options:

  1. Keep it as it is and hope importmaps don't take too long 😬
  2. Revert to <script src="js/three.min.js"></script> and update all the examples 🤷‍♂️

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 26, 2021

Revert to <script src="js/three.min.js"></script> and update all the examples 🤷‍♂️

Please not...

@mrdoob
Copy link
Owner

mrdoob commented Jan 26, 2021

Please not...

Why not? 🤔

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 26, 2021

I remember we discussed this topic in #20455 and it become apparent that the development community seems divided about this issue.

I still do think that working with global scripts is a bad thing, sorry. And we should stop doing it as soon as possible. My thoughts:

If you just need the core of three.js for an example, including three.min.js is of course nice and simple.

But remember the more complex examples like the post-processing ones. The imports looked awful in the early days. Because components like EffectComposer have dependencies to other files. And I think the user should not know about these dependencies nor handle it. And it should not be our task to bundle everything in EffectComposer only because to simplify the usage of global scripts.

At the time when three.js started, global scripts where something totally normal in the web development space because a proper native module system was not yet on the scope. However, let's not forget that using global scripts is associated with many conceptual problems like namespace pollution or missing dependency management.

Nowadays we have a module system in JavaScript and build systems like rollup. I believe it's more future-proof and robust to guide users to those technologies.

The most important thing is that manually downloading a library and copy it to a project directory is a No-Go in the module space. Unfortunately, import * as THREE from './js/three.module.js'; assumes the user does exactly that. Modules should never be copied by a dev.

I'm not sure how import maps will change this fact. AFAIK, they allow the usage of bare import specifiers and the configuration of the module resolution in the browser. However, users still have to define the mapping of bare import names to a relative or absolute URL. So it won't be the same like with global scripts.

Hence, I think it's best when users retrieve modules with a dependency manager or by using CDN URLs to a single host. I would use this pattern in the README file for now:

import * as THREE from 'https://unpkg.com/three/build/three.module.js';

In this way, the code matches to the respective live example.

@mrdoob
Copy link
Owner

mrdoob commented Jan 26, 2021

I'm not sure how import maps will change this fact.

Import maps allows using modules in a similar way to global scripts.

For example:

<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>

With import maps we can change OBJLoader, OrbitControls, ... so they do import { ... } from 'three'; instead of importing the library from ../../../build/three.module.js. This will allow the developer placing these files wherever.

The only path the developer would have to specify is the path for three.module.js.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 26, 2021

But what if the user downloads OBJLoader from r123, OrbitControls from r110 and three.module.js from the current release? Using npm would prevent this.

Besides, what happens if you include EffectComposer to this list? Would it be necessary to configure its dependencies (CopyShader, ShaderPass, MaskPass and ClearMaskPass) in the import map, too? Is it also necessary to manually download the dependencies?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 26, 2021

Sidenote: Maybe it's time to think about a three-cli npm module that supports the project setup and build with three.js.

> three new my-app
> cd my-app
> three serve

> three build my-app -c production // produces a bundle to dist/

@mrdoob
Copy link
Owner

mrdoob commented Jan 26, 2021

Interestingly, https://threejs-journey.xyz/ teaches using <script src="js/three.min.js"></script>, probably because it's more approachable to newcomers.

Correction... it doesn't. It's only the first lesson. They teach webpack.

@mrdoob
Copy link
Owner

mrdoob commented Jan 26, 2021

But what if the user downloads OBJLoader from r123, OrbitControls from r110 and three.module.js from the current release? Using npm would prevent this.

This came up some times over the years but not enough times to worry me.

Besides, what happens if you include EffectComposer to this list? Would it be necessary to configure its dependencies (CopyShader, ShaderPass, MaskPass and ClearMaskPass) in the import map, too? Is it also necessary to manually download the dependencies?

I know this is just one example but I was hoping to eventually deprecate EffectComposer and have renderer.setPostProcessing(). But yes, it is something we'll have to figure out case by case.

The thing is...

An important goal of mine was to make all this as approachable as possible for newbies but this is becoming harder and harder because experienced developers are building more a more tools that make newcomers learning curve stepper and stepper.

The first bump in the road was when it was no longer possible to use the library without running a web server. This period lasted around 5 years or so, and during that time examples/js made sense. These files were meant to be examples and people could just copy them and tweak for their needs.

Then commonjs became a thing, and we had to tweak a bunch of files so experienced developers could use the library on their projects. This period didn't last long.

Then es6 modules became a thing, we adopted them but now what's happening is that experienced devs are so used to the npm workflow that they would never think of copying files from examples/jsm and tweak for their use case. So their approach is to ask for features here and we no longer can say that these are just example files.

On top of that, es6 modules in the browser without importmaps is half baked. We realised this when we updated the examples to use modules. In order to make them work we had to do the ../../../build/three.module.js hack.

Now we have example modules that are a hack for both, experienced devs and newbies. I wish browsers had just waited for importmaps before adding es6 modules support. In retrospect, we adopted es6 modules in examples way too early.

At this point I'm starting to wonder if isn't just easier to:

  1. make the examples use global scripts again (or bet on importmaps and use a polyfill).
  2. use a bare specifier to import the library in /examples/jsm files
  3. move /examples/jsm to /addons (and generate examples/js from these).

I've been trying to think a solution for this for a while, but I'm not smart enough.
I'd be curious to hear about other approaches, but here's my wishlist:

  1. No bundling required. (ie. Just copy a couple of files and get a cube.)
  2. No Internet connection required. (ie. Working on a train with no internet.)

@mrdoob
Copy link
Owner

mrdoob commented Jan 26, 2021

If we bet on importmaps and adopt the polyfill then the code would look like this:

<script defer src="./js/es-module-shims.js"></script>
<script type="importmap-shim">
{
  "imports": {
    "three": "./js/three.module.js"
  }
}
</script>
<script type="module-shim">
    import * as THREE from 'three';
    import { OBJLoader } from '../addons/OBJLoader.js';
    import { OrbitControls } from '../addons/OrbitControls.js';

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

Or... We can revert to use global scripts in examples, and wait until importmaps are widely adopted.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 26, 2021

make the examples use global scripts again (or bet on importmaps and use a polyfill).

If only those two options would be available, I vote for the second one (import maps).

But yeah, this is a complicated topic and I feel I need more time to think about it in order to suggest a solution that is in some sense a good compromise for everybody.

@DefinitelyMaybe
Copy link
Contributor

[experienced developers doing more and requiring more which makes] newcomers learning curve stepper and stepper

Yup, but you could also say that was a good thing.

Place your bets everyone. Sounding like there's no simple good option.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 26, 2021

AFAICS, we all agree to generate examples/js from the modules. Maybe we can focus on this task as the next step?

Especially since there is already a PR trying to implement this feature (#20527). And solving this would also simplify the contribution and development process (see https://discourse.threejs.org/t/how-to-develop-in-examples-jsm-es6-vs-es5/22841).

@mrdoob
Copy link
Owner

mrdoob commented Feb 3, 2021

I was watching this video and found it "funny" that this actually works...

Screen Shot 2021-02-03 at 4 21 32 PM

Because the modules are trying to resolve this:

http://localhost:8000/../../build/three.module.js

😬

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

Successfully merging this pull request may close these issues.

4 participants