Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TensorFlow.js cannot be bundled because it imports a module seedrandom which is actually a CommonJS module #4003

Closed
jameshfisher opened this issue Oct 1, 2020 · 7 comments

Comments

@jameshfisher
Copy link
Contributor

System information:

  • Have I written custom code (as opposed to using a stock example script provided in TensorFlow.js): No
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): macOS 10.15.6
  • TensorFlow.js installed from (npm or script link): npm
  • TensorFlow.js version (use command below): 2.4.0
  • Browser version: recent Chrome

Describe the current behavior: ES module bundler complains when bundling TensorFlow.js, because it imports a module seedrandom which is actually a CommonJS module.

Describe the expected behavior: Running TensorFlow.js through an ES module bundler should not generate errors.

Standalone code to reproduce the issue: https://github.com/jameshfisher/tfjs-seedrandom-es-module-bug

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. I noted this here. 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 jameshfisher added the type:bug Something isn't working label Oct 1, 2020
@jameshfisher
Copy link
Contributor Author

This shows how I could "fix" the problem with rollup plugins: jameshfisher/tfjs-seedrandom-es-module-bug#1

But it's too gross for me to accept as a solution. I think I'll have to stick with plain <script> dependencies for now.

@tafsiri tafsiri added type:build/install type:support user support questions and removed type:bug Something isn't working labels Oct 1, 2020
@tafsiri
Copy link
Contributor

tafsiri commented Oct 1, 2020

@jameshfisher I saw on the issue you filed that David said he'd be willing to accept pull requests for this on the seedrandom side, so if you do end up making a PR there let us know and we can upgrade to an es friendly release!

@google-ml-butler
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 dyas if no further activity occurs. Thank you.

@google-ml-butler
Copy link

Closing as stale. Please @mention us if this needs more attention.

@google-ml-butler
Copy link

Are you satisfied with the resolution of your issue?
Yes
No

@jxmorris12
Copy link

I had this issue on some system importing tfjs from a webworker. It was easy to fix: npm i seedrandom --save

@rexemtoxa
Copy link

7 npm resolves all dependency correctly

yoyota added a commit to forward-head-posture/react-forward-head-posture that referenced this issue May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants