mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
103.03k stars 35.41k forks source link

Using DRACOLoader with bundlers #26403

Open cgauld opened 1 year ago

cgauld commented 1 year ago

Description

Hi!

We use the Parcel bundler extensively and, for the most part, the experience with three.js is fantastic. One issue we have encountered is with DRACOLoader - its dependencies (the worker JavaScript file and WASM file) are not detected by bundlers and thus you have to do some manual work to place the files in the right place or to reference externally hosted versions.

TLDR: We believe a small addition to DRACOLoader would make it much easier to work with these bundlers (including both Parcel and webpack). We're happy to submit a pull request with this addition - please let us know if it's welcome and we'll send it through πŸ™‚.

More technical details follow: These bundlers work by parsing the source files to build a graph of necessary assets. They then produce an output folder of optimized files for hosting. In order for dependencies to be detected, they must be referenced in ways that are supported by bunders. Both Parcel and webpack support the following structure for referencing raw files:

const wasmUrl = new URL('./mymodule.wasm', import.meta.url).toString();

With this structure, at build time the bundler will ensure mymodule.wasm is included in the final build output. It will also typically rename the file to include a content-based hash so the output can be indefinitely cached, so the final filename for the file may be something like mymodule-abcd1234.wasm. The bundler ensures that wasmUrl contains the correct URL at runtime for the file.

DRACOLoader requires some additional JavaScript and WASM files to work. At the moment, it has a setDecoderPath(path: string) function that you can use to say (at runtime) where those files are stored. The issue is that the bundlers don't support referencing a whole folder of files in this way and thus aren't able to produce an output folder that includes the necessary dependencies of DRACOLoader.

Our proposal, which matches what we've seen in some other libraries, including Emscripten itself, is to allow the user to provide an optional function to DRACOLoader, which it will call (if supplied) to retrieve the final URL for each of its assets. It would look something like this:

const loader = new DRACOLoader();
loader.resolveDependency = (filename: string) => {
    switch(filename) {
        case 'draco_decoder.wasm': return new URL('./node_modules/three/examples/jsm/lib/draco/draco_decoder.wasm', import.meta.url).toString();
        // etc
    }
};

This way DRACOLoader remains agnostic to whichever bundler you use (if any). To our knowledge, this would allow for config-less support in both Parcel and webpack.

We're happy to submit a pull request that adds this simple change to DRACOLoader - let if it's welcome πŸ™‚.

Solution

We'd like to provide a pull request that adds bundler support to DRACOLoader.

Alternatives

In most cases it's possible to manually copy the files over, but this can be problematic especially if you're providing a library or other self-contained code.

Additional context

No response

donmccurdy commented 1 year ago

Related:

This is a promising solution to the problems blocking earlier attempts... I think the approach should work?

/cc @gkjohnson @CodyJasonBennett

CodyJasonBennett commented 1 year ago

I'm not sure what this does differently, but I have no further concerns if CDNs play well with it.

@RenaudRohlinger I remember making a host of fixes offline to Vite for assets -- particularly with import.meta.url and its assets resolver. Does this ring a bell? I don't have access to our fork anymore. This is the only tool I recall having issues here, but they're good about backporting fixes if they're still needed.

gkjohnson commented 1 year ago

I would really like to see this happen for the loaders in a way that works across vanilla JS and build processes. Having to rely on CDNs or manual file configuration in builds feels a bit dirty.

const wasmUrl = new URL('./mymodule.wasm', import.meta.url).toString();

@cgauld Can you confirm that it's enough to have only a URL in this form in order for the referenced files to be included in the build? It doesn't need to be detected in an import, fetch, or Worker function call argument?

Generally having the above line as a default is safe. It should work in a web browser and in bundlers that support it - and it shouldn't cause systems that don't support it to choke assuming the user properly sets a custom path. In terms of bundler coverage it might be worth determining whether this works in ESBuild and Rollup, as well. Vite would be good to get working, as well. Otherwise it would be worth making some issues in the big bundlers to help promote some consistency in behavior here.

Dynamically importing the KTX or DRACO loaders is a separate problem that would be nice to address in the future.

cgauld commented 1 year ago

Hi @gkjohnson - I can confirm that the new URL(..., import.meta.url) structure is all that's required for Parcel and Webpack to include the file in the bundle. When this structure is used on its own, the bundlers will copy the file into the output bundle without performing any transformation (i.e. they will consider it to be a raw file and won't parse it for further dependency resolution or optimisation).

With my proposal we'd not include this structure in DRACOLoader itself - instead the loader would do exactly what it currently does but with one modification - it would call the resolveDependency function to get the URL to fetch for each given file (falling back to what it currently does if no resolveDependency function is provided by the user). That way the user (who knows which bundler they're using, if any) can use whichever structure works for them to ensure the file is included in the output and that DRACOLoader has a 'fetchable' URL for it. For webpack and parcel they'd implement a switch like the one above. Existing users wouldn't experience any change in behavior as long as they don't provide resolveDependency.

An alternative would be to update DRACOLoader to use this structure directly. That would have the benefit of working 'out of the box' for bundlers like Parcel and Webpack, without the user having to provide any resolveDependency function. The structure should also work directly in browsers, assuming the user places the dependencies beside the JS file they're calling DRACOLoader from. I don't have any direct experience with Vite but their documentation suggests it also supports the new URL... structure. In general though it seems likely there are users out there using different bundlers where it wouldn't work out of the box.

On balance, my recommendation would be the first option. It's a way that's agnostic to build processes and that allows users to work with whichever bundler they want. Perhaps in the future, when we're confident of support for the new URL structure across a broad spectrum of bundlers, a move to the second approach would make sense?

gkjohnson commented 1 year ago

In my strong opinion if this is going to be added the loader should use the URL( ... ) syntax by default so users don't have to set the decoder path. If this works in browsers and some of the most popular bundlers it should be the default. The existing API (or something like it) can be maintained for backwards compatibility and handling other types of bundler cases. Now that the examples and loaders all support es6 imports exclusively it's time to make this change.

cgauld commented 1 year ago

Cool - that definitely works for us πŸ˜„. I can pull a MR together that takes a shot at this if everyone is happy for me to do so? One element of the implementation that I should point out is that I think we should move DRACOLoader's dependencies from where they currently are (examples/jsm/libs/draco) to somewhere closer to the DRACOLoader itself (e.g. examples/jsm/loaders/draco). That way the the URL reference inside DRACOLoader will look something like this:

new URL('./draco/...', import.meta.url)

Which (as a path) will be much easier for those not using a bundler to replicate.

gkjohnson commented 1 year ago

One element of the implementation that I should point out is that I think we should move DRACOLoader's dependencies from where they currently are (examples/jsm/libs/draco) to somewhere closer to the DRACOLoader itself

I don't have strong feelings here but I expect this will be easier to merge without a discussion on how to rearrange the /libs directory. Maybe this can be done separately if you feel it's important.

cgauld commented 1 year ago

Hi! I've popped a draft PR here with these changes. I've left the libs where they are for the time being :-) Let me know your thoughts!

alexander-macleod commented 6 months ago

@cgauld any news on this? Perhaps a workaround that can be used? I'm also having a nightmare with Dracoloader and Parcel?

CodyJasonBennett commented 6 months ago

Don't ask for news on any OSS project. If there was any, it would live here in this issue. I'd continue with your question in the forum.

donmccurdy commented 6 months ago

I think the ideal outcome here would be if the Draco library itself β€” as published to npm β€” were web- and bundler-compatible. Unclear whether that will happen, but pressing πŸ‘ on this issue couldn't hurt:

As a workaround, I would just add that Meshopt compression does not have these bundler difficulties, and decodes very quickly.