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: Add 'import' section. #18778

Merged
merged 5 commits into from
Feb 8, 2023
Merged

Docs: Add 'import' section. #18778

merged 5 commits into from
Feb 8, 2023

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Feb 29, 2020

An idea to make the examples/js deprecation a bit easier. This would add a snippet to each example's page in the docs, showing how to import it from npm (default) or from a CDN.

Screenshot 2023-02-07 at 9 00 29 PM

Outdated screenshots

npm

npm

cdn

Screen Shot 2020-06-30 at 11 07 08 PM

Demo: https://raw.githack.com/donmccurdy/three.js/feat-unmodularize/docs/index.html#examples/en/loaders/GLTFLoader

@donmccurdy
Copy link
Collaborator Author

^This needs a little cleanup on mobile.

@munrocket
Copy link
Contributor

Interesting switch and converter. Also you hide node-fetch in new package.json.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 29, 2020

A utility is added to create the global script on demand, so this doesn't affect the build process.

Just for my understanding: Can you please explain why a separate script is necessary? Can't we just embedded a URL which links directly to https://unpkg.com/?

@donmccurdy
Copy link
Collaborator Author

@Mugen87 suppose examples/js goes away in r120: We can't link directly to https://unpkg.com/[email protected]/examples/js/loaders/GLTFLoader.js, because that script will be gone. So this lets you download the same thing from https://three-unmodularize.now.sh/[email protected]/examples/jsm/loaders/GLTFLoader.js, un-modularizing it dynamically from the ES Module hosted on unpkg.

We could do this without rehosting anything: download the ES module in page.js when the button is pressed, unmodularize it there, and initiate a file download. That'd be a bit simpler, but doesn't leave an easy way for users to get a particular version of the file.

I'm definitely fine with changing this, putting the download link somewhere less prominent than every single example page, or whatever.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 29, 2020

Thanks for the clarification! I've just misunderstood what https://unpkg.com/ actually does (I've thought it's a service that already performs the un-modularizing).

@donmccurdy
Copy link
Collaborator Author

There is one that does un-modularize: https://bundle.run/. But it runs Rollup and bundles dependencies (i.e. all of threejs) into the file, so doesn't really help us here. :/

@mrdoob
Copy link
Owner

mrdoob commented Apr 3, 2020

Looking good!
Do you mind unmodularize and docs change into different PR though?

@mrdoob mrdoob added this to the r116 milestone Apr 3, 2020
@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Apr 3, 2020

Sure – do you want the docs change to include the link to the downloadable file, or leave that to consider with a future unmodularize PR? Could also include a CDN like unpkg as an option I guess.

@mrdoob
Copy link
Owner

mrdoob commented Apr 3, 2020

Oh, I think I got confused. I thought the unmodularize script was to replace the modularize script so we could switch to update examples/jsm directly 🤔

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Apr 3, 2020

It is, sort of — it generates files like those have in js, from the jsm version. But I was assuming we wanted to delete examples/js entirely. So it doesn't write the files to disk, it just gives the user a temporary download of the examples/js-like file.

@donmccurdy
Copy link
Collaborator Author

Alright, this PR is just the docs portion now. It links to the unpkg CDN when the user clicks "Download", so for the moment there's no dependency on the unmodularize script.


@mrdoob Is it correct that we want to delete examples/js, and skip maintaining a reverse unmodularize.js script? What do you think of generating the un-modularized version dynamically like this? If we'd rather fully steer people away from that option, I guess a simpler version of this PR could include only ES modules.

@donmccurdy
Copy link
Collaborator Author

Checking in on this PR again, given #18749 — if we want this import section in the docs, should the section just describe ES module import? Or should it also provide a download of the old 'global' script generated on the fly (without keeping that global script in the GitHub repository and npm package)?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 27, 2020

I would prefer just to describe ES6 modules and not further promote the "old" way.

@donmccurdy
Copy link
Collaborator Author

Related: #19622

@mrdoob mrdoob modified the milestones: r118, r119 Jun 24, 2020
@donmccurdy donmccurdy marked this pull request as draft June 24, 2020 23:05
@donmccurdy
Copy link
Collaborator Author

I've reverted this to a draft for now — let me update the PR to match up with #19622 better.

@donmccurdy donmccurdy marked this pull request as ready for review July 1, 2020 06:08
@mrdoob mrdoob added this to the r149 milestone Dec 22, 2022
@mrdoob mrdoob modified the milestones: r149, r150 Jan 26, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 5, 2023

Any chances to rebase this regarding #25437? 😇

The PR could also be simplified, see #25437 (comment).

@donmccurdy
Copy link
Collaborator Author

@Mugen87 would you suggest we skip the dropdown, and just display something like this? Or keep the CDN option?

import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 6, 2023

Yes, I would skip the dropdown and displays paths like used in the Installation guide and examples so with the three/addons/* pattern. With the right import map, these paths work in node and the browser.

@epreston
Copy link
Contributor

epreston commented Feb 7, 2023

Hi guys, want me to get this done ? What order do you want these ?

Under the "Examples" heading or under the "Class/Component" main heading ?

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Feb 8, 2023

@Mugen87 I'd never intended to give advice about using Node.js, do you mean NPM? In any case, using the three/addons/* pattern sounds good to me, and I've updated the PR. No CSS or JS changes required in that case!

@epreston let's wait until this PR is approved and merged, but once the format and wording is OK I'd be glad to have help with updating the remaining pages, thanks! I think the Import section should come after the summary paragraphs, but above any other headings, including Examples.

@mrdoob mrdoob merged commit d440b3e into mrdoob:dev Feb 8, 2023
@donmccurdy donmccurdy deleted the feat-unmodularize branch February 8, 2023 02:36
@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

So green light to bang these out ?

Quick question, is there a section in the documentation that describes mapping examples/jsm to the alias three/addons in the users package.json file ?

I found that confusing when I first started. The distribution only contains an examples folder and like many, I spent an hour looking for the NPM package of three/addons. It's referred to in the installation guide as well.

Is this an example of what is to be inserted with the correct value to be supplied or am I missing something ? It is very rare for people to remap using the client packages.json into an npm imported package. The tools don't like it and complain because they often don't check it for NPM remapped imports.

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

The issue im describing is, copying the values from places like the install instructions or this PR does not work:

import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';

This is the value that will work if you copy and paste:

import { GLTFLoader } from 'three/examples/jsm/loaders/GLTFLoader.js';

To get the first one to work you need some trickery.

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

I see: you are describing it under the context of the HTML importmap that you create in HTML examples.

Inline importmaps is not the most common syntax (my first and only encounter is with this project).

It is really handy when you want to fit everything under one page, but theres a structure that is common between 'importmap', 'browser imports', and 'bundler imports'. It requires a small change but then code will work no matter where you copy and past it.

Update the 'importmap' to be: (I might need to tune this slightly)

"three/examples/jsm/": "https://unpkg.com/three@<version>/examples/jsm/"

This way if you create a separate 'main.js', use a bundler, or this inline importmap the copy and past is the same.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Feb 8, 2023

The package.json#exports entry is pretty widely supported in build tools these days, and that's what should generally be handling the three/addons resolution. This is far more common than using CDNs and import maps, in the wider web development world. If you are using a CDN you'll need an import map though, and that's what the link to Installation / Addons is there for. Hopefully that is enough — I'd prefer not to explain import maps on each of these pages if we can avoid it.

Both three/addons/ and three/examples/jsm/ would require an import map on a CDN, but I think we want to be consistent and stick to the first in documentation.

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

You're right. Because of the structure of this library, you need an 'importmap'. To get the pretty looking name three/addons/, it causes some confusion and requires extra config all over.

The exports config in this library's package.json does not create the alias in client code when using NPM.

This line does nothing for vscode or build tools.

"./addons/*": "./examples/jsm/*",

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

I may be doing a terrible job of describing it but it is a real issue.

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

You know what, I'm wrong. Please ignore.

This works just fine in NPM. I keep leaving the JSM in the path creating the issue.

"./addons/*": "./examples/jsm/*",

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

Shall I get started on the docs ? Don't want to step on toes, just help out if there's bits to be done.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Feb 8, 2023

There are some build tools that don't support package.json#exports mappings yet, notably Parcel, and these won't resolve three/addons. Those users may need to use three/examples/jsm instead, but it's a small issue now and getting smaller with time, so I don't think we need to burden these pages with it.

If you're willing to help copy these snippets into more pages that would be super, thank you! 🙏

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

It's on the way.

Pity we can't fix this up and drop importmap all together. I thought that was the idea behind dropping JS. I expected JSM to move into src as addons or something similar. Looks like the goal of all this is to have the examples point to the latest source to ease development and testing. Almost like something that should not go to the outside world.

@mrdoob
Copy link
Owner

mrdoob commented Feb 8, 2023

There are two types of web developers, the developers that use build tools (vite, webpack, etc) and the developers that don't (import maps).

The goal is to continue supporting both approaches.

@epreston
Copy link
Contributor

epreston commented Feb 8, 2023

Absolutely, just wish it was in a way that was less "unique" for the people consuming the library without a build tool.

I get the impression that there's a logical structure that accommodates all needs. Something that follows the same mental model as most packages.

You've got to see this as well. I mean, people raise issues about this. I don't remember seeing any dependencies of this project requiring import maps to get use of them. Once you get to the point of using a shim to support the structure it may point to using a different structure.

Maybe this is the best structure to accomodate developers using the library, people working on the library, tools manipulating the library, documentation running off the library.

I haven't done the work to know differently. If I do find something, I'll put in a PR.

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.

5 participants