ShaderParticleEngine icon indicating copy to clipboard operation
ShaderParticleEngine copied to clipboard

Latest master npm build broken because implicit THREE global is used

Open AndrewRayCode opened this issue 8 years ago • 5 comments

This is different from #94

Using the latest build on master:

require('shader-particle-engine-hotfix')
    ReferenceError: THREE is not defined

Currently the compiled main distributed source code uses an implied global THREE.

In the source code I think three should be required with commonjs syntax and included in package.json as a peerDependency. Or you could go the route taken by the OrbitControls npm project and make the user pass in THREE to some top level wrapper.

AndrewRayCode avatar Mar 16 '16 05:03 AndrewRayCode

This is fantastic, thanks Andrew!

A few points, though:

  • I've been considering refactoring this to make use of ES6 (mainly classes, import declarations, etc.), which would help somewhat in fixing up the currently quite haphazard NPM implementation. ES6 support is now (IMO) good enough to warrant having a pure ES6 build, alongside a Babel-compiled ES5 version.
  • I'm conscious of ensuring that those people who currently use the library without NPM aren't forced into using it as such, that it could still be used as a drop-in script.

I'll have a proper look over these PRs, but from the brief look I've just had, it seems like an interesting start :)

squarefeet avatar Mar 17 '16 13:03 squarefeet

It would be a very minor upgrade to switch from this to ES6 modules, you'd just have to change the require() calls in here to import/export.

AndrewRayCode avatar Mar 17 '16 16:03 AndrewRayCode

Also whatever builds the distributed bundle that's put into build/ will have the correct configuration to sort out the require() calls. npm users will use the normal entry flow and others will use the built file, which most js build tools now know how to work with commonjs

AndrewRayCode avatar Mar 17 '16 16:03 AndrewRayCode

Also I see you pushed 1.0.5 to npm, I would suggest not pushing to npm until this problem is fixed, because this package still fails with the error in the original ticket

AndrewRayCode avatar Mar 17 '16 19:03 AndrewRayCode

Having the same problem here using the NPM package 1.0.5 or 1.0.6. It's now a year later. Would it be possible to push a version to NPM without the need for the THREE (and SPE?) globals? It would be great to be able to use this nice thing through NPM!

Friksel avatar Jul 24 '17 11:07 Friksel