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

Bump swc #6848

Merged
merged 2 commits into from
Sep 15, 2021
Merged

Bump swc #6848

merged 2 commits into from
Sep 15, 2021

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Aug 31, 2021

Closes #6802
Closes #6765
Closes #6766
Closes #6719
Closes #6928

@height
Copy link

height bot commented Aug 31, 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.

@devongovett
Copy link
Member

Sourcemaps changed maybe?

@mischnic
Copy link
Member Author

mischnic commented Sep 1, 2021

v2:
Bildschirmfoto 2021-09-01 um 09 51 46

this:
Bildschirmfoto 2021-09-01 um 09 52 21

The return and the first quote character isn't mapped anymore

@devongovett
Copy link
Member

hmm is this bad? @DeMoorJasper

@DeMoorJasper
Copy link
Member

@devongovett from the screenshots it looks like the mappings are more complete now? But the text Niklas added says the opposite, so not sure? More mappings is always better (as long as they're accurate)

Looks fine to me either way, if it's less-complete we should probably file a bug report though

@devongovett
Copy link
Member

Oh ok, I think I misread it as well.

@danieltroger
Copy link
Contributor

Any progress on this?

@mischnic
Copy link
Member Author

mischnic commented Sep 8, 2021

I think I misinterpreted these results as well, the sourcemap is more accurate than before. So we'll probably only need to update the test fixtures

@danieltroger
Copy link
Contributor

Can confirm the sourcemaps were still wrong when I tried them out like two weeks ago so if this fixes them that'd be awesome.

Copy link

@findmet findmet left a comment

Choose a reason for hiding this comment

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

packages/transformers/js/core/Cargo.toml
And send for approval

@mischnic
Copy link
Member Author

mischnic commented Sep 11, 2021

Well, they still wrong:

Bildschirmfoto 2021-09-11 um 11 29 50

Bildschirmfoto 2021-09-11 um 11 29 56

They are mostly correct after removing tests/test/integration/sourcemap/.babelrc

@parcel-benchmark
Copy link

parcel-benchmark commented Sep 11, 2021

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.63s +13.00ms
Cached 255.00ms -15.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

React HackerNews ✅

Timings

Description Time Difference
Cold 8.42s -12.00ms
Cached 422.00ms +10.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 55.28s -139.00ms
Cached 1.51s -38.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.060c360f.js 1.78mb +62.00b ⚠️ 16.01s +11.00ms
dist/pdfRenderer.4b496bac.js 1.11mb +12.00b ⚠️ 42.15s -519.00ms
dist/index.051a8d66.js 694.71kb +10.00b ⚠️ 31.37s -11.22s 🚀
dist/editorView.f3e74e37.js 594.92kb -82.00b 🚀 31.37s +37.00ms
dist/index.749d3cbb.js 145.75kb +0.00b 31.37s -11.22s 🚀
dist/card.a2a4a158.js 62.67kb +0.00b 31.37s -11.22s 🚀
dist/card.45f850d6.js 5.77kb +0.00b 31.37s -11.22s 🚀
dist/Modal.7f4c60b3.js 3.15kb +0.00b 31.37s -11.22s 🚀
dist/media-card-analytics-error-boundary.b1a780f3.js 962.00b +0.00b 31.37s -11.22s 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/index.781e2635.js 1.78mb +72.00b ⚠️ 16.06s -302.00ms
dist/pdfRenderer.a6d29553.js 1.11mb +12.00b ⚠️ 43.16s -1.55s
dist/index.9d4aefdd.js 694.70kb +21.00b ⚠️ 43.11s -1.56s
dist/editorView.12a24514.js 594.92kb -82.00b 🚀 42.70s -1.92s
dist/EmojiPickerComponent.25aebafd.js 146.67kb +0.00b 26.47s +8.59s ⚠️
dist/Modal.8a291eba.js 45.33kb -15.00b 🚀 42.69s -1.92s
dist/esm.87ccebb6.js 33.15kb +0.00b 26.47s +8.59s ⚠️
dist/DatePicker.034a8928.js 22.98kb +0.00b 26.47s +8.59s ⚠️
dist/js.0f0bb621.js 17.25kb +0.00b 42.70s +9.82s ⚠️
dist/png-chunks-extract.94b5b9fc.js 3.58kb +0.00b 42.70s +9.82s ⚠️
dist/feedback.e18b45f0.js 1.77kb +0.00b 26.47s +8.59s ⚠️
dist/workerHasher.567cfc57.js 1.63kb +0.00b 26.47s +8.59s ⚠️
dist/heading6.b3b946d1.js 1.51kb +0.00b 26.46s +8.59s ⚠️
dist/heading5.e617db66.js 1.38kb +0.00b 26.46s +8.59s ⚠️
dist/expand.1f17de7f.js 1.29kb +0.00b 26.47s +8.59s ⚠️

Three.js ✅

Timings

Description Time Difference
Cold 5.83s -195.00ms
Cached 336.00ms -19.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@mischnic
Copy link
Member Author

I've removed the babelrc from that test to unblock this (and also because that most basic sourcemap test shouldn't involve babel + swc anyway)

@wbinnssmith wbinnssmith self-requested a review September 15, 2021 00:54
@devongovett devongovett merged commit 6f6759e into v2 Sep 15, 2021
@devongovett devongovett deleted the bump-swc branch September 15, 2021 04:45
lettertwo added a commit that referenced this pull request Sep 22, 2021
* v2:
  Upgrade Flow to 0.160.1 (#6964)
  Only use error overlay if there's a document (#6960)
  Don't fail when HTML tags are implied (#6752)
  Reorder resolveOptions() env priority (#6904)
  Change edge types to numbers (#6126)
  Bump swc (#6848)
  Print diagnostics for scope hoisting bailouts at verbose log level (#6918)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants