seedrandom icon indicating copy to clipboard operation
seedrandom copied to clipboard

NPM package does not provide an ECMAScript module

Open jameshfisher opened this issue 3 years ago • 6 comments

Context: I want to use TensorFlow.js in the browser. I'm using rollup to bundle my app for the browser.

The TensorFlow.js source imports from seedrandom in several places. This comes through in the ES modules distributed via npm.

To find the module seedrandom imported by TensorFlow, I'm using the plugin @rollup/plugin-node-resolve. This does find the node_modules/seedrandom package.

Unfortunately, @rollup/plugin-node-resolve does not find an ECMAScript module in the seedrandom package. So instead, it decides to bundle the seedrandom index.js. But this is a CommonJS module, not an ES module.

As a result, rollup gives me errors like alea is not exported by node_modules/seedrandom/index.js, and the browser gives me errors like require is not defined.

I think the right solution is for the seedrandom package to provide an ES module. The alternative is for TensorFlow to stop importing from a package that doesn't provide an ES module.

(You might say "well you just need to use a CommonJS->ES transpiler, like @rollup/plugin-commonjs". I'm reluctant to do so, because this transformation is gross, and in general, impossible.)

jameshfisher avatar Oct 01 '20 11:10 jameshfisher

Example: https://github.com/jameshfisher/tfjs-seedrandom-es-module-bug

jameshfisher avatar Oct 01 '20 11:10 jameshfisher

Pull request welcome!

On Thu, Oct 1, 2020, 7:17 AM Jim Fisher [email protected] wrote:

Example: https://github.com/jameshfisher/tfjs-seedrandom-es-module-bug

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/davidbau/seedrandom/issues/72#issuecomment-702065364, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2MN2ANXXNWEJOTEDACEU3SIRQK3ANCNFSM4SAITI7A .

davidbau avatar Oct 01 '20 13:10 davidbau

@jameshfisher, did you manage to get it working? I just came across the exact same issue and followed you here from https://github.com/tensorflow/tfjs/issues/4003.

alxAgu avatar Dec 17 '20 12:12 alxAgu

Unfortunately not and I haven't had time to work on a PR :( To work around this, I'm currently using the tensorflow.js browser bundle, rather than bundling it myself.

jameshfisher avatar Dec 17 '20 13:12 jameshfisher

I created a ESM module based fork called esm-seedrandom. The source is refactored to use ES6+ syntax to allow for treeshaking and to keep the minified output small and modular.

If someone is motivated, we can likely work the ESM fork back into patches for seedrandom using Rollup's various JS output to get CJS, UMD, and IIFE formats without breaking backward compatibility.

shanewholloway avatar Dec 23 '20 21:12 shanewholloway

@davidbau is there any chance of upstreaming the changes from @shanewholloway?

justinfagnani avatar Mar 22 '24 19:03 justinfagnani