filips123 commented 2 years ago

🐛 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

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

😯 Current Behavior

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.

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

josephshen commented 2 years ago

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

101arrowz commented 2 years ago

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.

danieltroger commented 1 year ago

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 commented 1 year ago

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

filips123 commented 10 months ago

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?

parcel-bundler / parcel

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