parcel icon indicating copy to clipboard operation
parcel copied to clipboard

Unnecessary `process` polyfill when checking `process.env` and incomplete treeshaking

Open filips123 opened this issue 2 years ago • 5 comments

🐛 Bug Report

Checking typeof process or process.env causes Parcel to unnecessarily polyfill process module, even if the code only uses process.env.SOME_ENV. Additionally, in 2.6.0, tree shaking does not work completely even without those checks.

I noticed this because I use semver package which contains this code:

const debug = (
  typeof process === 'object' &&
  process.env &&
  process.env.NODE_DEBUG &&
  /\bsemver\b/i.test(process.env.NODE_DEBUG)
) ? (...args) => console.error('SEMVER', ...args)
  : () => {}

It only uses process.env.NODE_DEBUG to determine whether debug should be enabled, so process should not be polyfilled and the whole function should be eliminated from production builds when debug is not enabled.

process is imported automatically from the process module, unless it's part of a process.browser or process.env.FOO expression which is replaced by a boolean or the value of the environment variable.

🎛 Configuration (.babelrc, package.json, cli command)

See repo in code sample section below.

🤔 Expected Behavior

  1. Parcel should not add unnecessary process polyfill when the code checks typeof process or process.env.
  2. Parcel should determine whether /\bsemver\b/i.test(process.env.NODE_DEBUG) is false and eliminate the whole function and its calls.

😯 Current Behavior

  1. Parcel thinks process module is used and adds process package polyfill:

    @parcel/resolver-default: Auto installing polyfill for Node builtin module "process"...
    
      D:\Users\filips\Downloads\parcel-polyfill-issue\src\index.js:4:10
        3 | const debug = (
      > 4 |   typeof process === 'object' &&
      >   |          ^^^^^^^ used here
        5 |   process.env &&
        6 |   process.env.NODE_DEBUG &&
    
      📝 Learn more: https://parceljs.org/features/node-emulation/#polyfilling-%26-excluding-builtin-node-modules
    
    @parcel/resolver-default: Auto installing polyfill for Node builtin module "process"...
    
      D:\Users\filips\Downloads\parcel-polyfill-issue\src\index.js:4:3
        3 | const debug = (
      > 4 |   process.env &&
      >   |   ^^^^^^^ used here
        5 |   process.env.NODE_DEBUG &&
        6 |   /\bsemver\b/i.test(process.env.NODE_DEBUG)
    
      📝 Learn more: https://parceljs.org/features/node-emulation/#polyfilling-%26-excluding-builtin-node-modules
    

    This happens both in 2.1.1 and 2.6.0, although it doesn't add process as a dependency in 2.1.1.

  2. Parcel 2.6.0 does not eliminate debug function completely when building for production, even if problematic checks are removed.

    Source:

    const debug = (
      process.env.NODE_DEBUG &&
      /\bsemver\b/i.test(process.env.NODE_DEBUG)
    ) ? (...args) => console.error('SEMVER', ...args)
      : () => {}
    
    debug('Hello World')
    

    Parcel 2.1.1 (works fine, empty file, except sourcemap comment):

    //# sourceMappingURL=index.81c000e3.js.map
    

    Parcel 2.6.0 (does not remove debug function):

    (()=>{})("Hello World");
    //# sourceMappingURL=index.16b57466.js.map
    

🔦 Context

In this case it adds a few KB to file size. However, it may add even more unneeded code in some cases, especially when treeshaking wouldn't work properly.

💻 Code Sample

  • Branch without checks: https://github.com/filips123/parcel-polyfill-issue/tree/process-without-check
  • Branch with checks: https://github.com/filips123/parcel-polyfill-issue/tree/process-with-check

🌍 Your Environment

Software Version(s)
Parcel 2.1.1, 2.6.0
Node 16.15.0
npm/Yarn 1.22.18
Operating System Windows 10

filips123 avatar May 28 '22 12:05 filips123

see https://github.com/parcel-bundler/parcel/pull/8158, ~~ ~~this bug was fixed if specified react-error-overlay to version 6.0.11~~ ~~ for your test code it seems they also introduced some bug this the process.env check

josephshen avatar May 28 '22 13:05 josephshen

This happens both in 2.1.1 and 2.6.0, although it doesn't add process as a dependency in 2.1.1.

I'm pretty sure it would've been an issue back in 2.1.1 as well but was not noticed because at that time Node built-ins were already Parcel dependencies, though I may be completely wrong on that.

This issue can probably be resolved by supporting typeof process within our constant expression evaluator.

101arrowz avatar Jun 13 '22 04:06 101arrowz

I'm looking for a way to make my library run both in bundlers that do node emulation and in bundlers that don't, I take from this issue that that's currently not possible?

This issue can probably be resolved by supporting typeof process within our constant expression evaluator.

This would be great.

danieltroger avatar Feb 01 '23 17:02 danieltroger

I'm getting this output with parcel 2.0.0-nightly.1347+824ddbe70 so I assume it's still an issue?

Screenshot 2023-08-01 at 09 45 08

danieltroger avatar Aug 01 '23 07:08 danieltroger

I've updated my code sample repository to Parcel 2.11.

TLDR: Part of this issue is still present.


The issue with installing polyfill is still present in Parcel 2.11.0:

@parcel/resolver-default: Auto installing polyfill for Node builtin module "process/"...

  node_modules\semver\internal\debug.js:2:10
    1 | const debug = (
  > 2 |   typeof process === 'object' &&
  >   |          ^^^^^^^ used here
    3 |   process.env &&
    4 |   process.env.NODE_DEBUG &&

  � Learn more: https://parceljs.org/features/node-emulation/#polyfilling-%26-excluding-builtin-node-modules

Installing process...

This can be seen when using semver package in the main branch.

A more minimal example that contains just the relevant code can be seen in the process-with-check branch. It also seems that the generated code in Parcel 2.11.0 is slightly longer than in 2.6.0. I don't know if that is considered to be an issue.


The issue with not eliminating debug function has been resolved. This can be seen in the process-without-check branch.


However, when using "alias": { "process": false } to disable the process polyfill, the dead code elimination still isn't working:

(({}).env,()=>{})("Hello World");

I've pushed an example to the process-with-check-disable-alias branch.

Should that be opened as a separate issue?

filips123 avatar Jan 28 '24 12:01 filips123