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

Examples: add three/addons/* alias #23406

Merged
merged 17 commits into from
Aug 25, 2022
Merged

Examples: add three/addons/* alias #23406

merged 17 commits into from
Aug 25, 2022

Conversation

marcofugaro
Copy link
Contributor

@marcofugaro marcofugaro commented Feb 1, 2022

Related issue: #23368 (comment)

Description

This allows us to import from examples like this:

import * as THREE from 'three';
import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';
import { VRButton } from 'three/addons/webxr/VRButton.js';

The advantage of this aliasing is that the imports of the examples are now the same both in node and the browser.

You can test this alias here https://raw.githack.com/marcofugaro/three.js/three/addons/examples/

Note: I used three/addons instead of three-addons as proposed here because in node,
import { ... } from 'three-addons/...' would import from the three-addons package instead of the three one.

@Mugen87 Mugen87 added this to the r138 milestone Feb 1, 2022
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 1, 2022

Please update the manual, too 😇 .

@marcofugaro
Copy link
Contributor Author

@Mugen87 done!

@abernier
Copy link
Contributor

abernier commented Feb 10, 2022

"addons" term makes so much more sense...

I'm quite new Three.js and really didn't understood why everything like CCDIKSolver was an "example", and not part of the reference documentation.

+1 for three/addons alias

@mrdoob
Copy link
Owner

mrdoob commented Feb 10, 2022

Note: I used three/addons instead of three-addons as proposed here because in node,
import { ... } from 'three-addons/...' would import from the three-addons package instead of the three one.

I think I would like to explore ways of publishing both three and three-addons from this repo...

I've also noticed that three-addons seems to be a bit outdated and the owner is someone we know 😁

Edit: three-addons seems easier to read but I wonder if it's worth the complication of publishing multiple packages...

@marcofugaro
Copy link
Contributor Author

marcofugaro commented Feb 10, 2022

I've also noticed that three-addons seems to be a bit outdated and the owner is someone we know 😁

😆😆😆

I think I would like to explore ways of publishing both three and three-addons from this repo...

Edit: three-addons seems easier to read but I wonder if it's worth the complication of publishing multiple packages...

@mrdoob Sure thing! Let's compare with the separate package solution, you decide which route is worth going forward (you're the one who is gonna publish 😬).

(I assume we want to keep all the code under this same repo still)

To publish multiple packages we would need to:

  1. stop publishing the examples/jsm folder on the three package
  2. add a package.json inside the examples/jsm
  3. optionally handle examples/jsm/renderers/nodes (add a package.json also there? and exclude it from the addons)
  4. handle examples/fonts (Bruno needs those)
  5. finally, document the new packages in the Installation guide

After this setup, when you need to publish a new version, you will need to publish each package. You can do this either:

  • by hand, cd into every folder and npm version minor && npm publish
  • with a script, bash or npm script, you would just run ./publish.sh or ./publish.sh 136
  • using some javascript tools like lerna, but we would need to restructure folders since lerna needs a root package.json for configuration

Let me know what do you think.

@abernier
Copy link
Contributor

abernier commented Feb 10, 2022

If using Lerna to publish multiple packages, a nice pattern (used by Lerna itself) could be to create a @three NPM "scope" to hold sub-packages, like:

three -- main package
@three/foo -- standalone foo scoped package
@three/bar -- standalone bar scoped package

@alexandernanberg
Copy link

three-addons seems easier to read but I wonder if it's worth the complication of publishing multiple packages...

Not worth it imo, and it makes it too easy for consumers to mess up. E.g. you'd need to make sure three and three-addons use the exact same version, otherwise you could run into various issues which people will create issues for.

@marcofugaro
Copy link
Contributor Author

hey @mrdoob, congrats on the release 🎉

Maybe we can get the three addons alias in this next cycle. You just have to tell me which option do you prefer:

  1. multiple package.json, publish by hand
  2. multiple package.json, publish with shell script ./publish.sh
  3. single package.json like it is now

@mattrossman
Copy link
Contributor

The advantage of this aliasing is that the imports of the examples are now the same both in node and the browser.

Silly question, but isn't this already possible with the existing three/examples/jsm pattern? What's the significance of adding a new name like three-addons or three/addons?

<script type="importmap">
{
  "imports": {
    "three": "https://unpkg.com/three@0.138.0/build/three.module.js",
    "three/examples/jsm/": "https://unpkg.com/three@0.138.0/examples/jsm/"
  }
}
</script>

Same imports with ESM: https://jsfiddle.net/mattrossman/zrLu2v6x/2/
And with a bundler: https://codesandbox.io/s/nervous-black-b4zq2u?file=/src/index.js

@alexandernanberg
Copy link

@mattrossman

Silly question, but isn't this already possible with the existing three/examples/jsm pattern?

Yeah, although I think three/addons is much more user friendly than three/examples/jsm. Especially when used with bundlers it feels strange to import from examples IMO

import { OrbitControls } from 'three/addons/controls/OrbitControls.js'
// vs
import { OrbitControls } from 'three/examples/jsm/controls/OrbitControls.js'

@mrdoob mrdoob removed this from the r139 milestone Mar 24, 2022
@mrdoob mrdoob modified the milestones: r141, r142 May 26, 2022
@mrdoob mrdoob modified the milestones: r142, r143 Jun 29, 2022
@LeviPesin
Copy link
Contributor

using some javascript tools like lerna, but we would need to restructure folders since lerna needs a root package.json for configuration

Why restructuring? Just one package.json in the root and one in the examples, and this should work?

@LeviPesin
Copy link
Contributor

Is this PR blocked only on the question should-we-and-if-should-then-how publish addons as a separate package?

I also think that in future when we remove examples/js, we can move examples/jsm to the root and rename the folder to addons (and this would not be a such breaking change if users start using three/addons alias now)...

@LeviPesin
Copy link
Contributor

Is this PR blocked only on the question should-we-and-if-should-then-how publish addons as a separate package?

I tend to think that we shouldn't do that. Or at least first merge this PR and then think about that...

@donmccurdy
Copy link
Collaborator

donmccurdy commented Aug 1, 2022

We really need to avoid three-* imports unless we are actually going to publish those NPM packages. If we're not sure, then it would be best to just use a safe alias like three/* in the meantime. I hope we can get either this PR or #24413 into one of the upcoming releases. 🙏

@marcofugaro
Copy link
Contributor Author

@mrdoob since #24413 has been merged, I'd advise we follow the same convention for the three/addons alias. 😇

Solved conflicts and updated the PR.

@LeviPesin
Copy link
Contributor

Please also note #24413 (comment).

@marcofugaro
Copy link
Contributor Author

marcofugaro commented Aug 25, 2022

@LeviPesin will do the examples/jsm folder in a new PR, after this gets merged. It's not fundamental for now.

@mrdoob mrdoob merged commit 034343d into mrdoob:dev Aug 25, 2022
@mrdoob
Copy link
Owner

mrdoob commented Aug 25, 2022

Thanks!

@LeviPesin
Copy link
Contributor

LeviPesin commented Sep 1, 2022

Hm, maybe we also should add such aliases for the libs? Like fflate instead of ../libs/fflate.module.js?

@donmccurdy
Copy link
Collaborator

I've been thinking of these aliases as shorthands to make our recommended workflows (namely, using ES Modules) more convenient for external users. But I consider examples/jsm/libs/* to be internal dependencies, which are also available as npm packages published by their original authors. Users do not as often need to refer to these packages directly, and when they do I'd encourage them to use the official npm packages instead. So I'm not sure we need aliases for files that are not really encouraged as a public API.

@LeviPesin
Copy link
Contributor

What if user just uses only files they needed, e.g. three.module.js, some loader using fflate, and fflate, and puts them in one folder (and does not use npm)? The user can change ../libs/fflate.module.js import to ./fflate.module.js or to change the directory structure and create libs, but I think that using fflate import in the loader and forcing user to just add fflate: ./fflate.module.js to the import map is simpler.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Sep 1, 2022

That is a workflow I'd prefer to discourage. By paving a path toward what is ultimately a fragile configuration, I think we are doing new users a disservice. three.js should offer one or two golden paths, supporting and documenting those well, and making it obvious when you're leaving that path — even if we don't ultimately require using npm.

As it is, our Installation page already gives more mixed messages about how to get started than I would like.

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
* Examples: use three/addons/* alias

* Npm: add three/addons/* alias

* Examples: remove unnecessary package.json

* Fix: remove build folder from PR

* Docs: use three/addons/ alias

* Editor: use three/addons/ alias

* Manual: use three/addons/ alias

* Manual: use three/addons/ alias in examples

* Add alias to new examples

* Add alias to new manual entries
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
* Examples: use three/addons/* alias

* Npm: add three/addons/* alias

* Examples: remove unnecessary package.json

* Fix: remove build folder from PR

* Docs: use three/addons/ alias

* Editor: use three/addons/ alias

* Manual: use three/addons/ alias

* Manual: use three/addons/ alias in examples

* Add alias to new examples

* Add alias to new manual entries
Mugen87 pushed a commit that referenced this pull request Mar 22, 2024
* Editor: use `three/addons/*` alias

Follow #23406, use more alias in editor

* fix: fix failed imports in last commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants