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

Address bug by updating an asset reference and merge conditions #8762

Merged
merged 4 commits into from
Jan 17, 2023

Conversation

AGawrys
Copy link
Contributor

@AGawrys AGawrys commented Jan 10, 2023

↪️ Pull Request

This pull request addresses issue-8716

To address this I've made sure merged assets are not seen as "bundleRoots" in any related structures, and made sure to update our assetReference structures which helps us map back nodes to the legacyGraph

💻 Examples

Test Case

✔️ PR Todo

  • Test on large app
  • Pass test from minimal repro
  • Included links to related issues/PRs

@parcel-benchmark
Copy link

parcel-benchmark commented Jan 11, 2023

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.89s -259.00ms 🚀
Cached 398.00ms -1.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 111.00ms -15.00ms 🚀
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 112.00ms -14.00ms 🚀
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 112.00ms -14.00ms 🚀
dist/legacy/index.2c76ad23.js 1.66kb +0.00b 532.00ms -124.00ms 🚀
dist/legacy/index.8aaa89c9.js 1.20kb +0.00b 532.00ms -123.00ms 🚀
dist/modern/index.6be20f01.js 1.13kb +0.00b 533.00ms -123.00ms 🚀
dist/legacy/index.html 826.00b +0.00b 671.00ms -169.00ms 🚀
dist/modern/index.html 749.00b +0.00b 670.00ms -169.00ms 🚀
dist/legacy/index.b8ae99ba.css 94.00b +0.00b 341.00ms -67.00ms 🚀
dist/modern/index.31cedca9.css 94.00b +0.00b 341.00ms -66.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 358.00ms +228.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 357.00ms +221.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 357.00ms +222.00ms ⚠️
dist/legacy/index.html 826.00b +0.00b 693.00ms -47.00ms 🚀
dist/modern/index.html 749.00b +0.00b 693.00ms -46.00ms 🚀

React HackerNews ✅

Timings

Description Time Difference
Cold 11.88s -97.00ms
Cached 530.00ms +12.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 2.17m +782.00ms
Cached 2.58s -38.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/media-viewer-analytics-error-boundary.6847f80f.js 3.43kb +0.00b 1.37m -16.50s 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/index.519d611b.js 3.94mb +0.00b 56.06s +3.03s ⚠️
dist/pdfRenderer.35745339.js 1.11mb +0.00b 1.42m +4.61s ⚠️
dist/editorView.601e3304.js 627.35kb +0.00b 1.74m +5.79s ⚠️
dist/refractor.a33d61f8.js 599.70kb +0.00b 1.74m +5.80s ⚠️
dist/media-viewer.3381b570.js 518.74kb +0.00b 1.42m +4.60s ⚠️
dist/popup.161ec9ff.js 322.41kb +0.00b 1.74m +5.80s ⚠️
dist/ConfigPanelFieldsLoader.71292279.js 312.70kb +0.00b 1.01m +3.42s ⚠️
dist/card.e5cee225.js 136.31kb +0.00b 1.42m +4.61s ⚠️
dist/EmojiPickerComponent.58efe218.js 95.42kb +0.00b 1.42m +4.61s ⚠️
dist/ConfigPanelFieldsLoader.c7f2aa0c.js 83.23kb +0.00b 1.42m +4.60s ⚠️
dist/mobile-upload.d7ac6521.js 68.12kb +0.00b 54.89s +2.86s ⚠️
dist/esm.ab7c7b7a.js 65.12kb +0.00b 1.74m +5.79s ⚠️
dist/ElementBrowser.05465fbe.js 62.44kb +0.00b 1.42m +4.61s ⚠️
dist/esm.e997ee63.js 61.48kb +0.00b 1.42m +4.61s ⚠️
dist/archive.b91410d8.js 61.47kb +0.00b 1.42m +4.61s ⚠️
dist/DatePicker.93c6d7f2.js 48.71kb +0.00b 1.01m +3.42s ⚠️
dist/component-lazy.1f0d85b0.js 48.50kb +0.00b 56.08s +3.04s ⚠️
dist/component.95bb1029.js 41.95kb +0.00b 54.90s +2.85s ⚠️
dist/esm.2613981d.js 40.66kb +0.00b 1.74m +5.80s ⚠️
dist/Modal.d50fae2a.js 28.71kb +0.00b 54.90s +2.85s ⚠️
dist/DatePicker.cb6a97f0.js 25.50kb +0.00b 1.01m +3.42s ⚠️
dist/smartMediaEditor.4380e25d.js 23.04kb +0.00b 1.74m +5.80s ⚠️
dist/esm.a4c15b2a.js 21.18kb +0.00b 1.74m +5.80s ⚠️
dist/component.19aeffe6.js 18.83kb +0.00b 54.89s +2.85s ⚠️
dist/js.59c58b0d.js 17.25kb +0.00b 54.89s +2.86s ⚠️
dist/ConfigPanelFieldsLoader.85392e43.js 16.22kb +0.00b 1.42m +4.60s ⚠️
dist/component.e1bd0f1e.js 15.48kb +0.00b 54.89s +2.86s ⚠️
dist/ui.fd196b0f.js 14.67kb +0.00b 1.42m +4.61s ⚠️
dist/dropzone.022632dd.js 13.91kb +0.00b 1.74m +5.79s ⚠️
dist/ConfigPanelFieldsLoader.35dd5c20.js 13.87kb +0.00b 1.42m +4.60s ⚠️
dist/dropzone.ad2bddc5.js 12.82kb +0.00b 1.74m +5.79s ⚠️
dist/component-lazy.6c68ff48.js 9.96kb +0.00b 56.08s +3.04s ⚠️
dist/Toolbar.f13c635f.js 9.69kb +0.00b 1.74m +5.80s ⚠️
dist/pdfRenderer.807762df.js 9.60kb +0.00b 1.42m +4.61s ⚠️
dist/clipboard.30b6a808.js 9.04kb +0.00b 1.74m +5.79s ⚠️
dist/browser.ff965d34.js 8.30kb +0.00b 1.74m +5.79s ⚠️
dist/mobile-upload.3b49b63c.js 8.11kb +0.00b 54.89s +2.86s ⚠️
dist/mobile-upload.327a0503.js 8.11kb +0.00b 1.42m +4.61s ⚠️
dist/mobile-upload.c045db2b.js 8.11kb +0.00b 1.42m +4.60s ⚠️
dist/mobile-upload.3aa19528.js 8.11kb +0.00b 1.74m +5.80s ⚠️
dist/index.b16227d6.css 4.08kb +0.00b 1.74m +5.79s ⚠️
dist/Modal.85101605.js 4.07kb +0.00b 54.89s +2.86s ⚠️
dist/png-chunks-extract.428c839f.js 3.46kb +0.00b 54.89s +2.86s ⚠️
dist/media-viewer-analytics-error-boundary.6847f80f.js 3.43kb +0.00b 1.42m -13.02s 🚀
dist/media-picker-analytics-error-boundary.6cc3986f.js 3.43kb +0.00b 1.74m +5.79s ⚠️
dist/media-card-analytics-error-boundary.fbc27a0b.js 3.43kb +0.00b 1.74m +5.80s ⚠️
dist/component.0685539f.js 3.40kb +0.00b 54.89s +2.86s ⚠️
dist/ru.fdb600e7.js 2.94kb +0.00b 1.42m +4.61s ⚠️
dist/uk.aeae0dd4.js 2.91kb +0.00b 1.42m +4.60s ⚠️
dist/codeViewerRenderer.83de4f0d.js 2.79kb +0.00b 1.42m +4.61s ⚠️
dist/th.bcd8fad5.js 2.75kb +0.00b 1.42m +4.61s ⚠️
dist/ResourcedEmojiComponent.bef36b85.js 2.68kb +0.00b 1.01m +3.42s ⚠️
dist/pl.4643976c.js 2.37kb +0.00b 1.01m +3.42s ⚠️
dist/cs.08737142.js 2.28kb +0.00b 1.01m +3.42s ⚠️
dist/de.c2e79abd.js 2.26kb +0.00b 1.01m +3.42s ⚠️
dist/es.983af340.js 2.24kb +0.00b 1.01m +3.42s ⚠️
dist/ja.b56014f3.js 2.24kb +0.00b 1.01m +3.42s ⚠️
dist/fr.aaa4d0bf.js 2.20kb +0.00b 1.01m +3.42s ⚠️
dist/pt_BR.6c08dcf7.js 2.17kb +0.00b 1.01m +3.42s ⚠️
dist/hu.3d2e30a0.js 2.17kb +0.00b 1.01m +3.42s ⚠️
dist/tr.3968c433.js 2.15kb +0.00b 1.42m +4.61s ⚠️
dist/vi.bf757a1c.js 2.15kb +0.00b 1.42m +4.60s ⚠️
dist/fi.080f52aa.js 2.13kb +0.00b 1.01m +3.42s ⚠️
dist/ko.51863560.js 2.13kb +0.00b 1.01m +3.42s ⚠️
dist/it.7bb93510.js 2.12kb +0.00b 1.01m +3.42s ⚠️
dist/nb.67163f41.js 2.10kb +0.00b 1.01m +3.42s ⚠️
dist/date.728667d8.js 2.10kb +0.00b 56.08s +3.04s ⚠️
dist/sv.51378563.js 2.09kb +0.00b 1.42m +4.61s ⚠️
dist/nl.336a2549.js 2.09kb +0.00b 1.01m +3.42s ⚠️
dist/da.f39b0c8c.js 2.07kb +0.00b 1.01m +3.42s ⚠️
dist/images.0fe0f35c.js 2.05kb +0.00b 56.09s +3.04s ⚠️
dist/zh_TW.6ca95d33.js 2.00kb +0.00b 1.42m +4.60s ⚠️
dist/zh.9b716de5.js 1.99kb +0.00b 1.42m +4.60s ⚠️
dist/feedback.2ade51a8.js 1.92kb +0.00b 1.01m +3.42s ⚠️
dist/status.2160efcd.js 1.82kb +0.00b 56.09s +3.04s ⚠️
dist/workerHasher.1e502a56.js 1.72kb +0.00b 54.89s +2.86s ⚠️
dist/workerHasher.0904bc5b.js 1.72kb +0.00b 1.42m +4.61s ⚠️
dist/workerHasher.6ae6406c.js 1.72kb +0.00b 1.42m +4.61s ⚠️
dist/workerHasher.23d0f86c.js 1.72kb +0.00b 1.74m +5.79s ⚠️
dist/workerHasher.6290622a.js 1.72kb +0.00b 1.74m +5.79s ⚠️
dist/workerHasher.e50d242f.js 1.72kb +0.00b 1.74m +5.80s ⚠️
dist/code.414e2c00.js 1.72kb +0.00b 56.08s +3.04s ⚠️
dist/list-number.2be0cd6c.js 1.62kb +0.00b 56.09s +3.04s ⚠️
dist/heading6.e0c2f3cf.js 1.52kb +0.00b 1.01m +3.42s ⚠️
dist/heading3.5b4663e8.js 1.51kb +0.00b 1.01m +3.41s ⚠️
dist/16.2ca688c7.js 1.48kb +0.00b 54.90s +2.85s ⚠️
dist/link.1e33efbb.js 1.43kb +0.00b 56.09s +3.04s ⚠️
dist/16.497f5b12.js 1.41kb +0.00b 54.89s +2.85s ⚠️
dist/emoji.5404dc09.js 1.40kb +0.00b 56.08s +3.04s ⚠️
dist/heading5.df611011.js 1.39kb +0.00b 1.01m +3.42s ⚠️
dist/expand.655beb16.js 1.33kb +0.00b 1.01m +3.42s ⚠️
dist/heading2.54f14b16.js 1.33kb +0.00b 56.09s +3.04s ⚠️
dist/heading4.3ae44c0a.js 1.28kb +0.00b 1.01m +3.42s ⚠️
dist/mention.85e501f2.js 1.24kb +0.00b 56.09s +3.04s ⚠️
dist/layout.dde5aa7a.js 1.20kb +0.00b 56.09s +3.04s ⚠️
dist/divider.3af42f3a.js 1.20kb +0.00b 56.08s +3.04s ⚠️
dist/action.633b1d3a.js 1.17kb +0.00b 56.08s +3.04s ⚠️
dist/heading1.216107b4.js 1.17kb +0.00b 56.09s +3.04s ⚠️
dist/list.1bd3e768.js 1.14kb +0.00b 56.09s +3.04s ⚠️
dist/quote.707178c1.js 1.14kb +0.00b 56.09s +3.04s ⚠️
dist/16.9b57f665.js 1.13kb +0.00b 54.89s +2.85s ⚠️
dist/decision.5c187f01.js 1.12kb +0.00b 56.08s +3.04s ⚠️
dist/panel-warning.817f153e.js 1.10kb +0.00b 56.09s +3.04s ⚠️
dist/16.58ff0dfa.js 1.08kb +0.00b 54.89s +2.85s ⚠️
dist/16.d4285343.js 1.08kb +0.00b 54.89s +2.85s ⚠️
dist/table.71e953fa.js 1.08kb +0.00b 56.09s +3.04s ⚠️
dist/16.1811e668.js 1.06kb +0.00b 54.89s +2.86s ⚠️
dist/16.095da4b1.js 1.06kb +0.00b 56.08s +3.04s ⚠️
dist/16.37c7d353.js 1.02kb +0.00b 56.08s +3.04s ⚠️
dist/panel.63b53aef.js 1.02kb +0.00b 56.09s +3.04s ⚠️
dist/panel-error.3c204bc9.js 1019.00b +0.00b 56.09s +3.04s ⚠️
dist/16.304b2149.js 991.00b +0.00b 54.90s +2.85s ⚠️
dist/16.88032cb6.js 963.00b +0.00b 54.90s +2.85s ⚠️
dist/panel-success.bfed2890.js 960.00b +0.00b 56.09s +3.04s ⚠️
dist/16.985ed0b8.js 956.00b +0.00b 54.89s +2.85s ⚠️
dist/16.d45c9dcb.js 950.00b +0.00b 56.08s +3.04s ⚠️
dist/panel-note.badaa0b2.js 950.00b +0.00b 56.09s +3.04s ⚠️
dist/16.7f99b86f.js 911.00b +0.00b 54.89s +2.85s ⚠️
dist/16.ff51ae00.js 905.00b +0.00b 54.89s +2.85s ⚠️
dist/16.0885132f.js 905.00b +0.00b 54.89s +2.85s ⚠️
dist/16.86eb7780.js 904.00b +0.00b 54.89s +2.85s ⚠️
dist/16.ff95e700.js 903.00b +0.00b 54.89s +2.85s ⚠️
dist/16.42ab2aa6.js 902.00b +0.00b 54.89s +2.86s ⚠️
dist/16.38996b14.js 875.00b +0.00b 56.08s +3.04s ⚠️
dist/16.bf3deacf.js 854.00b +0.00b 54.89s +2.85s ⚠️
dist/16.5e047b28.js 826.00b +0.00b 54.90s +2.85s ⚠️
dist/sk.dd4e83c8.js 791.00b +0.00b 1.42m +4.61s ⚠️
dist/pt_PT.1eda577d.js 786.00b +0.00b 1.01m +3.42s ⚠️
dist/et.03b90e09.js 778.00b +0.00b 1.01m +3.42s ⚠️
dist/simpleHasher.82f6b150.js 742.00b +0.00b 54.89s +2.86s ⚠️
dist/simpleHasher.2e0de700.js 742.00b +0.00b 1.42m +4.61s ⚠️
dist/simpleHasher.28ade3b5.js 742.00b +0.00b 1.42m +4.60s ⚠️
dist/simpleHasher.cc19c690.js 742.00b +0.00b 1.74m +5.79s ⚠️
dist/simpleHasher.76c4e98e.js 742.00b +0.00b 1.74m +5.80s ⚠️
dist/simpleHasher.46d6f2e5.js 742.00b +0.00b 1.74m +5.80s ⚠️
dist/is.0d0b2897.js 638.00b +0.00b 1.01m +3.42s ⚠️
dist/ro.82d888a1.js 633.00b +0.00b 1.42m +4.55s ⚠️
dist/en_GB.a4eaa606.js 623.00b +0.00b 1.01m +3.42s ⚠️
dist/en.dced70ab.js 620.00b +0.00b 1.01m +3.42s ⚠️
dist/index.html 167.00b +0.00b 1.74m +5.73s ⚠️

Three.js ✅

Timings

Description Time Difference
Cold 8.62s -213.00ms
Cached 320.00ms -34.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

bundles.set(movingAsset.id, mainNodeId);
}
replaceAssetReference(movingAsset, b, a);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably could merge this loop with the one below but not a big deal

Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on a couple apps and looks good.

@devongovett devongovett marked this pull request as ready for review January 17, 2023 20:41
@devongovett devongovett merged commit 7023c08 into v2 Jan 17, 2023
@devongovett devongovett deleted the bundle-css-module-bug branch January 17, 2023 22:10
marcins pushed a commit to marcins/parcel that referenced this pull request Jul 14, 2023
* upstream/v2: (33 commits)
  v2.8.3
  Changelog for v2.8.3
  Address bug by updating an asset reference and merge conditions (parcel-bundler#8762)
  Fix CSS order when merging type change bundles (parcel-bundler#8766)
  fixing failing build for contributors on Linux using Node 18 (parcel-bundler#8763)
  Extension: Importers View and separate LSP protocol package (parcel-bundler#8747)
  Bump swc to fix sourcemaps with Windows line endings (parcel-bundler#8756)
  Apply HMR updates in topological order (parcel-bundler#8752)
  Make extension packaging work (parcel-bundler#8730)
  Typed api.storeResult (parcel-bundler#8732)
  Refactor LSP to use vscode-jsonrpc (parcel-bundler#8728)
  Bump swc (parcel-bundler#8742)
  Recursively check reachability when removing asset graphs from bundles in deduplication (parcel-bundler#6004)
  Fix tsc sourcemaps metadata (parcel-bundler#8734)
  Assigning to `this` in CommonJS (parcel-bundler#8737)
  Don't retarget dependencies if a symbol is imported multiple times with different local names (parcel-bundler#8738)
  Add a note about using flow in CONTRIBUTING.md (parcel-bundler#8731)
  filter out title execArgv to workers (parcel-bundler#8719)
  Document more of the BundleGraph class (parcel-bundler#8711)
  Fixed the hmr connection with host 0.0.0.0 (parcel-bundler#7357)
  ...
lettertwo added a commit that referenced this pull request Nov 6, 2023
* upstream/v2: (128 commits)
  [webextension] Add support for `chrome_style` (#8867)
  Switch to SWC minifier by default (#8860)
  Use BitSet for bundler intersections (#8862)
  best key logic truncating package names (#8865)
  Add support for loadConfig to resolver plugins (#8847)
  Missing edge for multiple targets (#8854)
  Split large runtime manifest into separate bundles (#8837)
  Improvements to new resolver (#8844)
  Fix published files for resolver
  New resolver implementation in Rust (#8807)
  Update yarn.lock (#8843)
  Bump napi-rs to latest (#8838)
  Support .proxyrc.cjs  (#8833)
  Sort global deps before injecting imports (#8818)
  Sort CSS module exports (#8817)
  fix: add extra information to unique bundles (#8784)
  Don't blow up HMR when <link />s don't have hrefs (#8800)
  v2.8.3
  Changelog for v2.8.3
  Address bug by updating an asset reference and merge conditions (#8762)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants