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

Ensure named exports are prioritized over wildcard re-exports #7016

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Oct 5, 2021

Fixes #6344. Fixes #6970.

The ES module spec says that named exports should be prioritized over wildcard re-exports. This updates our helpers to skip keys that are already defined when performing a wildcard re-export. This is safe because we hoist named exports to the top of the module, so by the time the helper is called, all named exports are already defined.

Previously, we either errored when trying to read the previous value (non scope-hoisting), errored when trying to re-define the property (scope hoisting with multiple re-exports), or overwrote the value with the later one (scope hoisting). Now the value is consistent across all cases.

Note: duplicate wildcard re-exports is still not fully spec compliant, but we can address this separately. According to the spec, ambiguous re-exports should be completely omitted from the namespace object, but should error when performing a named import. We currently will return the first value instead. Unfortunately, it seems most tools are inconsistent in this regard.

@height
Copy link

height bot commented Oct 5, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@parcel-benchmark
Copy link

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 2.10s -14.00ms
Cached 295.00ms -35.00ms 🚀

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 661.00ms +580.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 662.00ms +581.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 661.00ms +580.00ms ⚠️
dist/legacy/index.8833cebd.js 1.59kb +0.00b 886.00ms -51.00ms 🚀
dist/legacy/index.01f0ce7e.js 1.20kb +0.00b 886.00ms -51.00ms 🚀
dist/modern/index.eb89329d.js 1.13kb +0.00b 885.00ms -51.00ms 🚀
dist/legacy/index.html 826.00b +0.00b 1.04s -63.00ms 🚀
dist/modern/index.html 749.00b +0.00b 1.03s -63.00ms 🚀
dist/legacy/index.5ba75694.css 94.00b +0.00b 1.15s -68.00ms 🚀
dist/modern/index.14235cd8.css 94.00b +0.00b 1.15s -68.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 699.00ms +68.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 700.00ms +68.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 700.00ms +67.00ms ⚠️

React HackerNews ✅

Timings

Description Time Difference
Cold 10.39s -58.00ms
Cached 477.00ms +13.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 1.16m +275.00ms
Cached 1.66s -3.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.94ff8fda.js 1.78mb +6.00b ⚠️ 19.93s -94.00ms
dist/index.4b4e2c61.js 694.70kb +15.00b ⚠️ 53.81s +567.00ms
dist/EmojiPickerComponent.428938a3.js 146.67kb +0.00b 21.83s -11.26s 🚀
dist/esm.0fd53840.js 33.15kb +0.00b 21.83s -11.26s 🚀
dist/DatePicker.2fc0068b.js 22.98kb +0.00b 21.83s -11.26s 🚀
dist/js.0f0bb621.js 17.25kb +0.00b 39.70s -13.07s 🚀
dist/png-chunks-extract.94b5b9fc.js 3.58kb +0.00b 39.70s -13.07s 🚀
dist/feedback.e18b45f0.js 1.77kb +0.00b 21.83s -11.26s 🚀
dist/workerHasher.567cfc57.js 1.63kb +0.00b 21.83s -11.26s 🚀
dist/heading6.b3b946d1.js 1.51kb +0.00b 21.83s -11.26s 🚀
dist/heading5.e617db66.js 1.38kb +0.00b 21.83s -11.26s 🚀
dist/expand.1f17de7f.js 1.29kb +0.00b 21.83s -11.26s 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/index.40bc0313.js 694.73kb -26.00b 🚀 53.02s -394.00ms
dist/editorView.a917034a.js 594.84kb +0.00b 52.97s +13.27s ⚠️
dist/popup.a4503db1.js 209.67kb +0.00b 52.97s +13.27s ⚠️
dist/Toolbar.fe910c25.js 107.23kb +0.00b 52.97s +13.27s ⚠️
dist/card.63c6c584.js 62.67kb +0.00b 52.97s +13.27s ⚠️
dist/Modal.5d4f8f78.js 45.33kb +0.00b 52.97s +13.27s ⚠️
dist/esm.aae5138c.js 33.15kb +0.00b 21.09s -1.06s 🚀
dist/ui.e9a3e4e8.js 14.94kb +0.00b 52.98s +13.27s ⚠️
dist/smartMediaEditor.7225054d.js 13.25kb +0.00b 52.97s +13.27s ⚠️
dist/dropzone.0d183115.js 12.15kb +0.00b 52.97s +13.27s ⚠️
dist/card.45f850d6.js 5.77kb +0.00b 52.97s +13.27s ⚠️
dist/EmojiPickerComponent.55e0a64c.js 3.73kb +0.00b 52.98s +13.27s ⚠️
dist/dropzone.cf53fd41.js 3.29kb +0.00b 52.97s +13.27s ⚠️
dist/Modal.7f4c60b3.js 3.15kb +0.00b 52.97s +13.27s ⚠️
dist/clipboard.78fcf485.js 2.93kb +0.00b 52.97s +13.27s ⚠️
dist/ResourcedEmojiComponent.78f6ddfc.js 2.12kb +0.00b 52.98s +13.27s ⚠️
dist/browser.09200cd5.js 1.69kb +0.00b 52.97s +13.27s ⚠️
dist/workerHasher.567cfc57.js 1.63kb +0.00b 21.09s -1.06s 🚀
dist/status.f223f976.js 1.59kb +0.00b 21.09s -1.05s 🚀
dist/heading6.b3b946d1.js 1.51kb +0.00b 21.09s -1.05s 🚀
dist/media-card-analytics-error-boundary.75fac461.js 1.12kb +0.00b 52.97s +13.27s ⚠️
dist/media-picker-analytics-error-boundary.cfe02cc6.js 966.00b +0.00b 52.97s +13.27s ⚠️
dist/media-card-analytics-error-boundary.b1a780f3.js 962.00b +0.00b 52.97s +13.27s ⚠️
dist/simpleHasher.2a6e12b6.js 643.00b +0.00b 52.98s +13.27s ⚠️

Three.js ✅

Timings

Description Time Difference
Cold 7.66s -6.00ms
Cached 443.00ms +28.00ms ⚠️

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@fatoyeus
Copy link

@mischnic what action do I need to take to have this fix?

@mischnic
Copy link
Member

mischnic commented Oct 27, 2022

Use any version of Parcel 2.

@fatoyeus
Copy link

@mischnic With regards to parcel I have below and still experiencing same error.

"dependencies": {
"@auth0/auth0-react": "^1.10.2",
"@mswjs/data": "^0.8.4",
"@parcel/diagnostic": "2.7.0",
"@parcel/plugin": "2.7.0",
"@parcel/source-map": "^2.0.0",
"@parcel/utils": "2.7.0",
"@parcel/workers": "2.7.0",
.........
}

"devDependencies": {
"parcel": "latest"
}

@mischnic
Copy link
Member

Please open a new issue with a complete code example to reproduce the error (and you didn't actually say exactly what the error is that you're getting).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Material-UI Failing in Parcel 2.0.0-rc.0 export * should not override named exports that occur before
4 participants