Used code incorrectly dropped with sideEffects: false & unrelated reexport

[Andarist]

🐛 bug report

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

{
  "scripts": {
    "build:parcel": "parcel build ./src/index.html --dist-dir dist/parcel"
  }
}

🤔 Expected Behavior

Used import should stay in the bundle and be called correctly

!function(){var o,s=Object.prototype.hasOwnProperty;console.log((o={css:""},void(s.call(o,"css")||console.log("has css prop"))))}();

😯 Current Behavior

Used import is being removed and replaced with {}

console.log(void({}.call({css:""},"css")||console.log("has css prop")));

💁 Possible Solution

This most likely happens during scope hoisting / module concatenation but I can't be totally sure because I don't know enough about Parcel's architecture and, unfortunately, I haven't more time digging into this right now.

What I have been able to establish is that the problem is somewhere before treeShake gets called.

🔦 Context

We got a report about @emotion/react being broken with Parcel 2 here: emotion-js/emotion#2114 (comment)

It results in a runtime error for production builds, therefore the impact on users is severe.

💻 Code Sample

I've prepared quite a detailed repro case here:
https://github.com/Andarist/parcel-sideeffects-flag-bug-repro

When bundling the entrypoint this line from a dependency is affected:
https://github.com/Andarist/parcel-sideeffects-flag-bug-repro/blob/fd386c2c9c2f01ff3ec6135e4e2f0672e05a61dc/src/node_modules/emotion-react/index.js#L5

and the hasOwnProperty gets replaced with {} which leads to a runtime error in a production build. The final output can be checked here:
https://github.com/Andarist/parcel-sideeffects-flag-bug-repro/blob/fd386c2c9c2f01ff3ec6135e4e2f0672e05a61dc/dist/parcel/index.f7c7b3c5.js#L1

I've also included webpack and Rollup demos for comparison (they both work correctly):
https://github.com/Andarist/parcel-sideeffects-flag-bug-repro/tree/fd386c2c9c2f01ff3ec6135e4e2f0672e05a61dc/dist

What I have also been able to establish is that this happens because:

• sideEffects: false is specified for this dependency
• there is an unrelated reexport from the root entry of the dependency that can be successfully removed because it stays unused

You can check out 2 demos that remove each one of them (but not both at the same time!):

• without sideEffects: false: https://github.com/Andarist/parcel-sideeffects-flag-bug-repro/compare/without-sideeffects-false
• without reexport: https://github.com/Andarist/parcel-sideeffects-flag-bug-repro/compare/without-reexport

They both lead to the same, correct, output - they result in the very same hash in the generated JS bundle.

🌍 Your Environment

Software	Version(s)
Parcel	2.0.0-nightly.447
Node	12
npm/Yarn	yarn 1.19
Operating System	OSX Mojave

[mischnic]

Seems to be fixed in the current nightly (2.0.0-nightly.454)?

[Andarist]

Oh, you are right. I have focused on creating the minimal repro case and I haven't checked the very latest nightly build 🤦 This is already fixed in 2.0.0-nightly.452 and my bet is that this got fixed by #4861 . I'm going to close this issue and check if there is a test covering this specific situation - if not I will try to provide a PR with the appropriate regression test.

[mischnic]

👍

Honestly at first I though this would be the first bug report for something introduced by #4861.

---

I think this is similar to this one? https://github.com/parcel-bundler/parcel/blob/v2/packages/core/integration-tests/test/integration/scope-hoisting/es6/re-export-import/b.js

The problem being that there there is first an import and then an unused reexport to the same file. Then the export would mark the dependency as "weak".
With that PR, now individual symbols (like hasOwnProperty and useTheme in this case) are marked weak.

[Andarist]

This test looks quite similar (I'm talking purely about the structure of the linked file) but this is not a test added in #4861 so I assume that there is some difference between it and the case presented here. Otherwise, the test suite would be already failing for a long time.

I'm not sure if I have understood your later comment - but if you are saying that the export+import order has been important for my case then it's not true. I've rechecked with both orders and the result is broken in the very same way (on the 2.0.0-nightly.447)

[mischnic]

but if you are saying that the export+import order has been important for my case then it's not true.

Yeah, the order doesn't really matter. isWeak was false by default then an reexport statement would set it to true. Imports didn't change the value.

parcel/packages/shared/scope-hoisting/src/hoist.js

Line 639 in 296bc57

isWeak: true,

---

This test is very similar: https://github.com/parcel-bundler/parcel/blob/v2/packages/core/integration-tests/test/integration/scope-hoisting/es6/side-effects-re-exports-import/node_modules/bar/index.js. Import+reexport, the reexport is unused, sideEffects: false

[Andarist]

It indeed looks very similar (as the other one that you have pointed me to) - but this is also not a new test 🤔

[mischnic]

-> #5376