"sideEffects" annotation in package.json may be out of date

[evanw]

I'm working on a bug filed by a user of this library against esbuild, a bundler I work on. It looks to me like this library uses the sideEffects annotation in package.json to indicate that most files in the package do not have side effects. However, they do actually have side effects and this annotation causes the resulting bundle to be broken.

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.7
• Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device: N/A
• TensorFlow.js installed from (npm or script link): npm
• TensorFlow.js version (use command below): ?
• Browser version: Chrome 80
• Tensorflow.js Converter Version: N/A

Describe the current behavior

I tested this by bundling this example code from your readme:

// Import @tensorflow/tfjs or @tensorflow/tfjs-core
import * as tf from '@tensorflow/tfjs';
// Adds the WASM backend to the global backend registry.
import '@tensorflow/tfjs-backend-wasm';
// Set the backend to WASM and wait for the module to be ready.
tf.setBackend('wasm').then(() => main());

function main() { console.log('works') }

Bundling that with webpack ./example.js (Webpack version 5.4.0, the current one) and running the bundle in the browser with a script tag throws Error: Backend name 'wasm' not found in registry.

Describe the expected behavior

I expect works to be printed to the console.

Suggested fix

The easiest way to fix this is to remove the sideEffects field from your package.json files. This should make sure the library remains correct when bundled.

A harder way to fix this would be to go through all of your files and figure out which ones have side effects, and then include them in that list. For example, at least the file dist/backend_wasm.js should be in there because it contains a call to registerBackend('wasm'). This is harder because you will have subtle correctness issues if you later on change your files (introduce new ones, move code between files, etc.) and forget to update the sideEffects field correctly.

Standalone code to reproduce the issue

Additional files used:

• package.json

  {
    "scripts": {
      "build": "webpack ./index.js"
    },
    "dependencies": {
      "@tensorflow/tfjs": "2.7.0",
      "@tensorflow/tfjs-backend-wasm": "2.7.0",
      "@tensorflow/tfjs-core": "2.7.0",
      "webpack": "5.4.0",
      "webpack-cli": "4.2.0"
    }
  }

• index.html

  <script src="dist/main.js"></script>

[tafsiri]

Thanks for reporting this, I'll take a look. We initally set this up to work with webpack 4 but can check on the behaviour in webpack 5. At least at that point we needed to have the explicit sideEffects list present to get the behaviour we want.

[evanw]

Thanks for taking a look! I'm sorry about this class of problems being an issue. Ideally sideEffects would have been specified as opt-in instead of opt-out because that would have been much less error-prone, but that ship has unfortunately sailed already.

One more thing to point out is that it's not just the @tensorflow/tfjs-backend-wasm package. For example, here's a warning that esbuild generates when bundling the @tensorflow/tfjs-core package:

 > node_modules/@tensorflow/tfjs-core/dist/flags.js: warning: Ignoring this import because "node_modules/@tensorflow/tfjs-core/dist/engine.js" was marked as having no side effects
     17 │ import './engine';
        ╵        ~~~~~~~~~~
       node_modules/@tensorflow/tfjs-core/package.json: note: It was excluded from the "sideEffects" array in the enclosing "package.json" file
    121 │   "sideEffects": [
        ╵   ~~~~~~~~~~~~~

Presumably the import './engine' statement is trying to import that file just for its side effects, but then package.json instructs the bundler to treat the file as if it has no side effects when all of the imports are unused.

[tafsiri]

Thanks for the detailed report @evanw, We missed a few files in our sideEffects listing. I've made a PR to address this and unblock this issue, and will be testing out webpacks new ability to do without the sideEffects field more extensively. It would be great if we don't have to track this manually, but will have to make the call as to whether we need to support webpack 4.

[google-ml-butler]

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