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

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

Open
filips123 opened this issue May 28, 2022 · 5 comments
Labels
🐛 Bug Good First Issue 🐡 JS Codegen Stale Ignore This issue is exempt from getting flagged as stale and autoremoved

Comments

@filips123
Copy link

🐛 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

🌍 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
Copy link

josephshen commented May 28, 2022

see #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
Copy link
Member

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.

@github-actions github-actions bot added the Stale Inactive issues label Dec 10, 2022
@mischnic mischnic added Good First Issue Stale Inactive issues 🐡 JS Codegen 🐛 Bug and removed Stale Inactive issues labels Dec 10, 2022
@parcel-bundler parcel-bundler deleted a comment from github-actions bot Dec 10, 2022
@mischnic mischnic removed the Stale Inactive issues label Dec 10, 2022
@danieltroger
Copy link
Contributor

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.

@github-actions github-actions bot added the Stale Inactive issues label Aug 1, 2023
@danieltroger
Copy link
Contributor

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

@github-actions github-actions bot removed the Stale Inactive issues label Aug 1, 2023
@github-actions github-actions bot added the Stale Inactive issues label Jan 28, 2024
@filips123
Copy link
Author

filips123 commented Jan 28, 2024

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?

@github-actions github-actions bot removed the Stale Inactive issues label Jan 28, 2024
@github-actions github-actions bot added the Stale Inactive issues label Jul 27, 2024
@parcel-bundler parcel-bundler deleted a comment from github-actions bot Jul 27, 2024
@mischnic mischnic added Stale Ignore This issue is exempt from getting flagged as stale and autoremoved and removed Stale Inactive issues labels Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Good First Issue 🐡 JS Codegen Stale Ignore This issue is exempt from getting flagged as stale and autoremoved
Projects
None yet
Development

No branches or pull requests

5 participants