Skip to content

feat(build)!: Drop pre-ES2020 polyfills #14882

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

Merged
merged 3 commits into from
Jan 7, 2025
Merged

feat(build)!: Drop pre-ES2020 polyfills #14882

merged 3 commits into from
Jan 7, 2025

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 2, 2025

This PR updates to use a forked version of sucrase (https://github.com/getsentry/sucrase/tree/es2020-polyfills).
On this branch, a new option disableES2019Transforms is added, which can be used to only polyfill ES2020 features:

  • numeric separators (e.g. 10_000)
  • class fields (e.g. class X { field; })

It also adds a lint step that checks all build output for ES2020 compatibility, to avoid regressions in this regard.

@mydea mydea self-assigned this Jan 2, 2025
Copy link
Contributor

github-actions bot commented Jan 2, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.79 KB +0.04% +9 B 🔺
@sentry/browser - with treeshaking flags 21.54 KB +0.04% +8 B 🔺
@sentry/browser (incl. Tracing) 35.38 KB +0.04% +14 B 🔺
@sentry/browser (incl. Tracing, Replay) 72.1 KB -0.64% -475 B 🔽
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.6 KB -0.58% -368 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas) 76.36 KB -0.79% -616 B 🔽
@sentry/browser (incl. Tracing, Replay, Feedback) 88.89 KB -0.51% -458 B 🔽
@sentry/browser (incl. Feedback) 39.59 KB +0.09% +34 B 🔺
@sentry/browser (incl. sendFeedback) 27.45 KB +0.1% +27 B 🔺
@sentry/browser (incl. FeedbackAsync) 32.22 KB +0.08% +26 B 🔺
@sentry/react 25.55 KB +0.02% +5 B 🔺
@sentry/react (incl. Tracing) 38.18 KB +0.03% +11 B 🔺
@sentry/vue 27.07 KB +0.03% +7 B 🔺
@sentry/vue (incl. Tracing) 37.24 KB +0.04% +15 B 🔺
@sentry/svelte 22.96 KB +0.05% +10 B 🔺
CDN Bundle 24.18 KB +0.04% +8 B 🔺
CDN Bundle (incl. Tracing) 35.73 KB +0.05% +15 B 🔺
CDN Bundle (incl. Tracing, Replay) 70.27 KB -0.67% -482 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 75.51 KB -0.59% -452 B 🔽
CDN Bundle - uncompressed 70.71 KB -0.05% -30 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 106.16 KB -0.05% -45 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.04 KB -1.41% -3.1 KB 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 229.92 KB -1.34% -3.1 KB 🔽
@sentry/nextjs (client) 38.28 KB -0.53% -207 B 🔽
@sentry/sveltekit (client) 35.91 KB +0.05% +16 B 🔺
@sentry/node 162.1 KB -0.5% -821 B 🔽
@sentry/node - without tracing 97.92 KB -0.76% -767 B 🔽
@sentry/aws-serverless 127.73 KB -0.61% -802 B 🔽

View base workflow run

mydea added a commit that referenced this pull request Jan 3, 2025
We already have an eslint rule to avoid class fields, but had exceptions
for static fields as well as for arrow functions.

This also leads to bundle size increases, so removing the exceptions and
handling the (few) exceptions we have there should save some bytes.

Additionally, this has additional challenges if we want to avoid/reduce
polyfills, as class fields need to be polyfilled for ES2020, sadly.

Found as part of
#14882
mydea added 2 commits January 3, 2025 11:00
@mydea mydea requested review from AbhiPrasad, lforst and Lms24 January 3, 2025 10:16
@mydea mydea marked this pull request as ready for review January 3, 2025 10:18
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

One question: Should we to adjust the makeSucrasePlugin call in bundleHelpers.,js as well?

@@ -144,7 +146,8 @@
"resolutions": {
"gauge/strip-ansi": "6.0.1",
"wide-align/string-width": "4.2.3",
"cliui/wrap-ansi": "7.0.0"
"cliui/wrap-ansi": "7.0.0",
"**/sucrase": "getsentry/sucrase#es2020-polyfills"
Copy link
Member

Choose a reason for hiding this comment

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

Gotta admit, I had no idea that one can simply specify a repo/branch name here. Nice!

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, it required us to ensure that the dist folder is actually checked in on that branch, but IMHO this appears easier than to publish stuff from there etc 😅

Copy link
Member

Choose a reason for hiding this comment

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

for sure!

@mydea
Copy link
Member Author

mydea commented Jan 3, 2025

makeSucrasePlugin

It should be the same now in all places, as we just generally set this option always?

@mydea mydea changed the title feat(build)!: Drop polyfills & bump ES target to ES2020 feat(build)!: Drop pre-ES2020 polyfills Jan 3, 2025
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

It should be the same now in all places, as we just generally set this option always

Oh right, missed that, sorry! Let's :shipit: then!

@mydea mydea merged commit e48ffef into develop Jan 7, 2025
156 checks passed
@mydea mydea deleted the fn/update-build branch January 7, 2025 12:19
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.

None yet

3 participants