Making 'three' tree-shakeable

[pschroen]

The goal with tree-shaking here is I should be able to import a class from 'three', and only get the classes needed. In my test bundle importing only Vector2 is producing a 295 KB (uncompressed) file still with lots of remaining side-effects even after r141 and all the work done on #24006 (down from a 1 MB bundle in r140).

I'm opening this issue to resolve the remaining side-effects, which is do-able with some more work, and we have a couple ways of testing that.

Also to make the claim that 'three' is finally fully tree-shakeable, we'll need to verify that in the most popular bundlers. I'll start with Rollup, Webpack, Parcel and esbuild. Open to suggestions of other bundlers and configurations, and will start with a simple test of importing only Vector2.

Steps to reproduce the behavior:

import { Vector2 } from 'three';

console.log(new Vector2(0, 1));

And with agadoo:

npx agadoo
...
Failed to tree-shake build/three.module.js

It's worth noting that importing from the source files with agadoo also fails, and is something I can look into as well.

npx agadoo src/Three.js
...
Failed to tree-shake src/Three.js

The expected behavior, regardless of agadoo, is simply looking at the output bundle. If I import Vector2, I'm expecting only the @license header comment, and Vector2, nothing else.

References:

• https://twitter.com/MarcoGomezGT/status/1520866649485754372
• Tree-shaking broken by Object.defineProperties( ... ) #24006

[marcofugaro]

Other related discussions: #21667 #21665

[pschroen]

@mrdoob @Mugen87 @donmccurdy @LeviPesin I'd also like to use this issue to discuss the use of instanceof, does anyone have an example of it not working?

My fear here is that something has been misunderstood with how tree-shaking works and has led us all on a wild goose chase. Would be great to get some insight from either @Rich-Harris or @lukastaegert on this? See #24006 (comment).

In my experience it's always been working for me, in Rollup at-least (and I've been using it since 2016). For example, I've been using my own TextureLoader, and when bundling my own classes in with other libraries the bundler should be renaming conflicting object names so when they are instantiated instanceof will always be comparing the correct constructor class:

• TextureLoader becomes TextureLoader$1.
• So the actual code output in the bundle would be loader instanceof TextureLoader$1, which works.
• Code minification also works so long as the object names all match.

If there is an issue here, in my opinion that would actually be an issue with the bundler or minification, not with the use of instanceof.

Related discussion: #24167

[LeviPesin]

Does instanceof work with tree-shaking?
I.e. if, for example, WebGLRenderer has .isPointsMaterial check in its body but does not have PointsMaterial import, does the changing it to instanceof PointsMaterial and importing PointsMaterial will break tree-shaking or not? I.e. will the PointsMaterial class be included?
Related: #24006 (comment)

[pschroen]

Oh I think I get why this might be an issue, is the problem simply that PointsMaterial would be included in the bundle?

Technically it is being used in that case, and would be included in the bundle. If the goal is to not include any of the materials with WebGLRenderer, then ya we would have to use .isPointsMaterial.

I'll add this to my list of things to investigate...

[LeviPesin]

Oh I think I get why this might be an issue, is the problem simply that PointsMaterial would be included in the bundle?

Yes.

If the goal is to not include any of the materials with WebGLRenderer, then ya we would have to use .isPointsMaterial.

I think we should calculate how often such pattern is used (i.e. the class is not imported, but .isClass is used)...

[marcofugaro]

I.e. if, for example, WebGLRenderer has .isPointsMaterial check in its body but does not have PointsMaterial import, does the changing it to instanceof PointsMaterial and importing PointsMaterial will break tree-shaking or not? I.e. will the PointsMaterial class be included?

Yup, it will be included. This is the reason three switched to .is* in #9310.

[LeviPesin]

And there is a more detailed explanation in #9310 (comment).

BTW (completely unrelated to this issue but related to that), is the reasons for not using default exports valid now? They are very useful to avoid constantly writing import { Abc } and export { Abc } (they are already used in e.g. the nodes system).

[marcofugaro]

BTW (completely unrelated to this issue but related to that), is the reasons for not using default exports valid now? They are very useful to avoid constantly writing import { Abc } and export { Abc } (they are already used in e.g. the nodes system).

Code style I believe.

[LeviPesin]

And there is a more detailed explanation in #9310 (comment).

In that comment the following link was given:
#4776 (comment)
(It is said there that polymorphism can be used instead of .is checks)
I think we can use the same pattern in most such occurences?

[pschroen]

is the reasons for not using default exports valid now?

Code style I believe.

Agree with @marcofugaro, it's an opinionated topic as a result.

Using named exports is supposed to be friendlier for tree-shaking when you are always explicitly using the named imports, but in practice I've found it makes no difference in my experience, the bundler renames all the objects anyways, and I use both default exports and named exports, though over the years I've moved more towards named exports/imports for consistency and being more explicit.

[LeviPesin]

I think we can use the same pattern in most such occurences?

E.g. the first occurence of .is that GitHub search shows - https://github.com/mrdoob/three.js/blob/dev/src/renderers/shaders/UniformsUtils.js - can be easily fixed by changing the value.isVector2 || ... to value.clone !== undefined. Will file a quick PR for this.

[donmccurdy]

Sorry I don't see how changing obj.isPointsMaterial to obj instanceof PointsMaterial could ever benefit tree-shaking. Using instanceof will certainly create a reference to the class and prevent tree-shaking, and .is does not...