Add side effect detection visitor
↪️ Pull Request
This is a first pass on a side effect detecting visitor that can upgrade assets in an unknown side effect state to sideEffects: false in some cases.
Running it on a very large code base, it detected 33% of assets as side effect free. I haven't found any false positives but feedback and more test coverage should give more confidence.
Major changes in this PR
- Added a new side effects visitor to the core JS transformer
- Only runs in scope hoisting mode (due to being reliant on import local names)
- Only runs when the Asset is in an unknown side effects state
- Added a new property (
hasResolvedSideEffects) to Assets to be able to distinguish betweensideEffects: trueandsideEffects: undefined- The new property allows for backwards compatibility
- Updates node-resolver-core to only return side effects in known situations, otherwise it will be undefined.
- Previously we were defaulting in both the resolver and on the core Asset
💻 Examples
🚨 Test instructions
✔️ PR Todo
- [x] Added/updated unit tests for this change
- [x] Filled out test instructions (In case there aren't any unit tests)
- [ ] Included links to related issues/PRs
Benchmark Results
Kitchen Sink ✅
Timings
| Description | Time | Difference |
|---|---|---|
| Cold | 1.51s | +8.00ms |
| Cached | 273.00ms | -24.00ms 🚀 |
Cold Bundles
| Bundle | Size | Difference | Time | Difference |
|---|---|---|---|---|
| dist/legacy/parcel.7cdb0fad.webp | 102.94kb | +0.00b | 243.00ms | +13.00ms ⚠️ |
| dist/legacy/parcel.7cdb0fad.webp | 102.94kb | +0.00b | 251.00ms | +15.00ms ⚠️ |
| dist/modern/parcel.7cdb0fad.webp | 102.94kb | +0.00b | 251.00ms | +14.00ms ⚠️ |
| dist/legacy/index.b8ae99ba.css | 94.00b | +0.00b | 252.00ms | +15.00ms ⚠️ |
| dist/modern/index.31cedca9.css | 94.00b | +0.00b | 252.00ms | +15.00ms ⚠️ |
Cached Bundles
No bundle changes detected.
React HackerNews ✅
Timings
| Description | Time | Difference |
|---|---|---|
| Cold | 4.11s | -67.00ms |
| Cached | 376.00ms | -2.00ms |
Cold Bundles
No bundle changes detected.
Cached Bundles
| Bundle | Size | Difference | Time | Difference |
|---|---|---|---|---|
| dist/PermalinkedComment.3145598b.js | 3.94kb | +0.00b | 420.00ms | +56.00ms ⚠️ |
| dist/UserProfile.b37bbaff.js | 1.38kb | +0.00b | 419.00ms | +55.00ms ⚠️ |
| dist/NotFound.c08212ea.js | 265.00b | +0.00b | 420.00ms | +56.00ms ⚠️ |
| dist/logo.8dd07848.png | 244.00b | +0.00b | 326.00ms | +24.00ms ⚠️ |
AtlasKit Editor ✅
Timings
| Description | Time | Difference |
|---|---|---|
| Cold | 37.87s | -1.15s |
| Cached | 2.18s | -0.00ms |
Cold Bundles
| Bundle | Size | Difference | Time | Difference |
|---|---|---|---|---|
| dist/media-viewer.bc1a2415.js | 537.32kb | +0.00b | 11.81s | +2.59s ⚠️ |
| dist/ConfigPanelFieldsLoader.8648eeee.js | 306.81kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/card.3521c96b.js | 140.18kb | +0.00b | 8.31s | -910.00ms 🚀 |
| dist/mobile-upload.0917d4f0.js | 66.50kb | +0.00b | 5.31s | -285.00ms 🚀 |
| dist/ElementBrowser.c496dd44.js | 62.20kb | +0.00b | 8.31s | -910.00ms 🚀 |
| dist/archive.fe044de4.js | 60.16kb | +0.00b | 11.81s | +2.59s ⚠️ |
| dist/esm.ce3e12df.js | 59.72kb | +0.00b | 8.32s | -900.00ms 🚀 |
| dist/component-lazy.aeb22f50.js | 59.50kb | +0.00b | 6.24s | +337.00ms ⚠️ |
| dist/component.508d12ab.js | 57.88kb | +0.00b | 5.30s | -291.00ms 🚀 |
| dist/DatePicker.f4cb448f.js | 47.85kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/Modal.f90b31a7.js | 28.20kb | +0.00b | 5.31s | -270.00ms 🚀 |
| dist/DatePicker.412226fe.js | 25.02kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/component.d038388b.js | 18.68kb | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/js.9cb9c5be.js | 17.21kb | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/ConfigPanelFieldsLoader.8efb299e.js | 15.82kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/ui.8e1e1200.js | 14.49kb | +0.00b | 8.31s | -910.00ms 🚀 |
| dist/ConfigPanelFieldsLoader.f78f3b60.js | 13.65kb | +0.00b | 8.31s | -910.00ms 🚀 |
| dist/pdfRenderer.6335b9a2.js | 12.08kb | +0.00b | 11.56s | +2.35s ⚠️ |
| dist/mobile-upload.86840439.js | 7.86kb | +0.00b | 5.30s | -272.00ms 🚀 |
| dist/mobile-upload.c687ddb2.js | 7.86kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/mobile-upload.e9eb996a.js | 7.86kb | +0.00b | 8.32s | -902.00ms 🚀 |
| dist/Modal.efe95f7f.js | 3.87kb | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/component.342752e9.js | 3.22kb | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/media-viewer-analytics-error-boundary.54c54975.js | 3.19kb | +0.00b | 11.81s | +2.59s ⚠️ |
| dist/png-chunks-extract.01ed8f60.js | 3.06kb | +0.00b | 5.30s | -272.00ms 🚀 |
| dist/ru.aaea8ba6.js | 2.81kb | +0.00b | 8.31s | +1.70s ⚠️ |
| dist/uk.5d2e97bd.js | 2.76kb | +0.00b | 8.31s | -912.00ms 🚀 |
| dist/codeViewerRenderer.7d374cd5.js | 2.61kb | +0.00b | 11.79s | +2.57s ⚠️ |
| dist/th.df60823c.js | 2.60kb | +0.00b | 8.31s | -912.00ms 🚀 |
| dist/ResourcedEmojiComponent.184d62aa.js | 2.47kb | +0.00b | 6.25s | -359.00ms 🚀 |
| dist/pl.f089a702.js | 2.25kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/cs.c0d356c1.js | 2.23kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/de.1a167b65.js | 2.17kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/fr.6cc5b166.js | 2.12kb | +0.00b | 6.25s | -359.00ms 🚀 |
| dist/es.38a88442.js | 2.12kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/hu.026ff8dd.js | 2.10kb | +0.00b | 6.25s | -359.00ms 🚀 |
| dist/fi.84541eb7.js | 2.09kb | +0.00b | 6.25s | -359.00ms 🚀 |
| dist/ja.a9cd0bd6.js | 2.09kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/vi.3e6d5bcb.js | 2.09kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/pt_BR.1db6fd92.js | 2.06kb | +0.00b | 7.65s | +1.04s ⚠️ |
| dist/tr.4de346b9.js | 2.03kb | +0.00b | 8.31s | -912.00ms 🚀 |
| dist/ko.954590a1.js | 1.97kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/sv.b893ead3.js | 1.97kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/it.5c7edaaf.js | 1.97kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/nb.7f52770f.js | 1.96kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/date.6db71354.js | 1.94kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/da.23f674ea.js | 1.94kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/nl.fd54481e.js | 1.94kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/images.21df3a8f.js | 1.90kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/zh_TW.3d130b76.js | 1.85kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/zh.fb21f066.js | 1.83kb | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/feedback.647089cf.js | 1.76kb | +0.00b | 6.25s | -361.00ms 🚀 |
| dist/status.be4e3842.js | 1.67kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/code.64a301f3.js | 1.56kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/workerHasher.e01f8bcf.js | 1.56kb | +0.00b | 5.30s | -272.00ms 🚀 |
| dist/workerHasher.322762e4.js | 1.56kb | +0.00b | 8.31s | -910.00ms 🚀 |
| dist/workerHasher.8fdadeba.js | 1.56kb | +0.00b | 8.32s | -903.00ms 🚀 |
| dist/list-number.e454dc8e.js | 1.47kb | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/heading6.eae34279.js | 1.36kb | +0.00b | 6.25s | -361.00ms 🚀 |
| dist/16.a4c7368c.js | 1.35kb | +0.00b | 5.31s | -272.00ms 🚀 |
| dist/heading3.82217cc7.js | 1.35kb | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/16.347f2ad3.js | 1.28kb | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/link.ef87b7d4.js | 1.28kb | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/emoji.f9caa19f.js | 1.25kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/heading5.20183aa6.js | 1.23kb | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/expand.e7437f2e.js | 1.18kb | +0.00b | 6.25s | -359.00ms 🚀 |
| dist/heading2.a43a84af.js | 1.17kb | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/heading4.bc1ea347.js | 1.12kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/mention.12d040af.js | 1.08kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/layout.467fc22b.js | 1.04kb | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/divider.875eeb9b.js | 1.04kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/action.4747cf93.js | 1.02kb | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/heading1.a2a2d506.js | 1.01kb | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/16.1d939d76.js | 1.00kb | +0.00b | 5.30s | -276.00ms 🚀 |
| dist/list.a024d070.js | 1007.00b | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/quote.790784b0.js | 1007.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/decision.f09ec841.js | 988.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/16.92be0d97.js | 976.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/16.5befcdea.js | 976.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/panel-warning.b246d7d3.js | 964.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/16.ef2df2b6.js | 956.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/16.792a9556.js | 951.00b | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/table.348d2eb0.js | 942.00b | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/16.01cdc55d.js | 916.00b | +0.00b | 5.31s | -285.00ms 🚀 |
| dist/panel.bc621c8f.js | 883.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/panel-error.9152f129.js | 860.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/16.8fc349c9.js | 858.00b | +0.00b | 5.31s | -271.00ms 🚀 |
| dist/16.c5423dbe.js | 830.00b | +0.00b | 5.31s | -285.00ms 🚀 |
| dist/16.1be49b4b.js | 823.00b | +0.00b | 5.30s | -276.00ms 🚀 |
| dist/16.eb6f51c1.js | 817.00b | +0.00b | 5.58s | -316.00ms 🚀 |
| dist/panel-success.25629fed.js | 801.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/panel-note.f23dd251.js | 791.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/16.924dd2e3.js | 778.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/16.01e372d3.js | 772.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/16.0cd21a5f.js | 772.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/16.a0490963.js | 771.00b | +0.00b | 5.30s | -277.00ms 🚀 |
| dist/16.a5f45cdb.js | 770.00b | +0.00b | 5.30s | -275.00ms 🚀 |
| dist/16.bc1a05f3.js | 769.00b | +0.00b | 5.30s | -276.00ms 🚀 |
| dist/16.d9cd1f88.js | 742.00b | +0.00b | 5.58s | -315.00ms 🚀 |
| dist/16.e3d16653.js | 721.00b | +0.00b | 5.30s | -276.00ms 🚀 |
| dist/16.592b5fd3.js | 693.00b | +0.00b | 5.31s | -285.00ms 🚀 |
| dist/sk.1a0c584e.js | 652.00b | +0.00b | 8.31s | -910.00ms 🚀 |
| dist/pt_PT.16308ef8.js | 631.00b | +0.00b | 7.65s | +1.03s ⚠️ |
| dist/et.3d28125f.js | 629.00b | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/simpleHasher.329400f6.js | 585.00b | +0.00b | 5.30s | -272.00ms 🚀 |
| dist/simpleHasher.0488d56a.js | 585.00b | +0.00b | 8.31s | -911.00ms 🚀 |
| dist/simpleHasher.180c1d91.js | 585.00b | +0.00b | 8.32s | -903.00ms 🚀 |
| dist/is.b5f0121f.js | 491.00b | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/ro.ee42c980.js | 478.00b | +0.00b | 8.23s | +1.61s ⚠️ |
| dist/en_GB.f6c48dd5.js | 468.00b | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/en.b8f14ffb.js | 465.00b | +0.00b | 6.25s | -360.00ms 🚀 |
| dist/index.html | 248.00b | +0.00b | 11.96s | +5.41s ⚠️ |
Cached Bundles
| Bundle | Size | Difference | Time | Difference |
|---|---|---|---|---|
| dist/media-viewer.bc1a2415.js | 537.32kb | +0.00b | 9.16s | -3.28s 🚀 |
| dist/archive.fe044de4.js | 60.16kb | +0.00b | 9.16s | -3.28s 🚀 |
| dist/index.html | 248.00b | +0.00b | 6.47s | -5.99s 🚀 |
Three.js ✅
Timings
| Description | Time | Difference |
|---|---|---|
| Cold | 3.15s | +16.00ms |
| Cached | 313.00ms | -5.00ms |
Cold Bundles
No bundle changes detected.
Cached Bundles
No bundle changes detected.
@marcins On the webpack tests, I actually couldn't find any tests for their logic. 🤷
@mattcompiles We discussed with @mischnic in the meeting today. Sounds like we will need a new value/mode for asset.sideEffects that indicates that the side effects only apply to that asset and not to any dependencies. The packager already has the ability to skip an asset while preserving the asset's dependencies (shouldSkipAsset), and also a mode that skips entire subgraphs (isDependencySkipped). I think we just need something similar in symbol propagation.