creativelifeform / three-nebula

WebGL based particle system engine for three.js
https://three-nebula.org/
MIT License
945 stars 76 forks source link

three.nebula.js containing a copy of all three.js code makes three.js included twice in webpack bundle #54

Closed revyTH closed 5 years ago

revyTH commented 5 years ago

Hi, I have noticed that since three-nebula package has a dependency on three.js, bundling applications that also have root dependencies to three.js causes three.js library to be included twice in the final bundle, one for the depedency on three-nebula and one for root dependencies on three.js.

Considering for example the following package.json dependencies:

"dependencies": {
    "three": "^0.104.0",
    "three-nebula": "^4.0.3",
  },

And the following js file:

import { Scene, PerspectiveCamera, Mesh } from "three"

import System, {
    Emitter,
    Rate,
    Span,
    Position,
    Mass,
    Radius,
    Life,
    Velocity,
    PointZone,
    Vector3D,
    Alpha,
    Scale,
    Color,
} from 'three-nebula'

Building this file using WebPack results in a file of 1.4 MB (threejs library packed alone is about 650 KB).

To include three.js only once in the bundle, the thing that works for me was to modify the file three-nebula/src/core/three.js to point directly to the root package ./node_modules/three/build/three.module.js (instead of ./node_modules/three-nebula/node_modules/three) and make imports from ./node_modules/three-nebula/src/index.js instead. In that case the final bundle size is around 800 KB.

rohan-deshpande commented 5 years ago

Hey, thanks for this issue - it's definitely been in the back of my mind.

I also had a huge bug with two instances of three being potentially used in apps and this caused WebGL context problems. I resolved this via the core/three internal module but that doesn't fix your concern.

I've got two options here

  1. Export three from three-nebula meaning that consumers no longer need to include three as a dependency in their app, they can simply import { THREE } from 'three-nebula'. This feels quite unnatural though
  2. Attempt to remove the three package dependency entirely by simply cloning the required classes internally within the library. This will work, but will be a bit of a chore to keep things up to date with each release of three. It does give me the smallest slice of code needed however.

Out of the two, I'm leaning towards the second option. Some of the usages of three classes are just for instanceof equality checks, since each class in the library has a type property, these can be substituted.

There's a bunch of others that I won't be able to do this for though because they are hard dependencies.

I need to have a think about it. For now, please use your solution, I'll try to come up with a way to do this soon so that this is solved for future consumers.

Edit: Also noted that you are using three@0.104.0, currently only three@0.98.0 is supported. Things will probably work, but I can't guarantee it.

revyTH commented 5 years ago

Hi, for my use case it is working fine with 104.

You may want to look at https://github.com/vanruesc/postprocessing, it uses option 2 I guess so could give some hint.

Cheers and thanks for the beautiful library!

rohan-deshpande commented 5 years ago

Thanks, I have a full particle designer that uses this as its engine coming out soon. Would love to hear about your use case for this lib and what you’re doing with it!

I’ll pick this issue up soon.

revyTH commented 5 years ago

Great! I am using the library as stars particle system in a webgl website: https://ludik.herokuapp.com/#/

rohan-deshpande commented 5 years ago

This is awesome! Looks really good

rohan-deshpande commented 5 years ago

@revyTH https://github.com/creativelifeform/three-nebula/pull/59 I'll release this under a major version alpha as there are core API changes, but the bundle size has gone down to around 100kb

rohan-deshpande commented 5 years ago

@revyTH have released this change under v5.0.0-alpha, would you mind trying it out and seeing if it fixes your issue? I can probably squeeze a few more kb out of things but would just like to see if it's alleviated most of the pain. I've also updated the compatible three version to r106.

You may need to make some changes in your code as the fromJSON & fromJSONAsync methods as well as the BodySprite, MeshRenderer and SpriteRenderer classes now all require THREE to be passed as an arg.

bummzack commented 5 years ago

Wouldn't it be better to just use ES6 modules instead? That means to just import from "three", eg. import { SpriteMaterial } from "three".

That way, module-bundlers would only include the required modules once… so the problem would be immediately solved for people that use ES6 modules.

For users relying on the bundled version of Three.js, some fallback to the THREE global should be implemented. I guess this can be achieved with some webpack settings, for example declaring Three.js as external in the webpack config (so that it will never be bundled with your build) and maybe use something like the define plugin to switch between ES6 imports and the global THREE.

rohan-deshpande commented 5 years ago

This is how it was previously but it exploded the bundle size. I want the library to work in any context with minimal setup and to have as small a size as possible. The bundle size has been reduced greatly in the latest release. If there’s a way to get it working with the much cleaner approach as before, I’d be open to it! I really hate webpack though and want as little to do with it as possible. Would be really keen to see a PR with these changes if you feel it’s simple enough.

Edit: Btw I'm going to close this issue for now, I'll open a new one and link to your comment @bummzack