GoogleChromeLabs / tooling.report

tooling.report a quick way to determine the best build tool for your next web project, or if tooling migration is worth it, or how to adopt a tool's best practice into your existing configuration and code base.
https://tooling.report
Apache License 2.0
848 stars 49 forks source link

Add test for evaluating environment variables away #350

Closed nstepien closed 4 years ago

nstepien commented 4 years ago

https://github.com/facebook/react/blob/master/packages/react-dom/npm/index.js#L31 Many packages use process.env.NODE_ENV to ship less/optimized code for prod builds, it'd be nice to have a test for this. This works fine in webpack, but last time I tried I had trouble with rollup, as I could only replace code as strings, so it wouldn't work well with more dynamic code, for example const { NODE_ENV } = process.env;.

jakearchibald commented 4 years ago

Isn't this use-case already covered by https://bundlers.tooling.report/transformations/flags/?

nstepien commented 4 years ago

Ah you're right, I hadn't seen that. Though for rollup's case @rollup/plugin-replace only works like a string replacement (+ patching sourcemaps), rather than evaluating process.env as a static object.

What I ended up doing in my rollup config was:

    plugins: [
      replace({
        'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV)
      }),

      // ...

      isProduction && (await import('rollup-plugin-terser')).terser({
        ecma: 2020,
        compress: { global_defs: { process: undefined } } // remove remaining references to Node's `process`
      })
    ],

Terser's global_defs setting helped remove any remaining usage of process that replace couldn't take care of.