Repository navigation
chore(app): replace core-js with local shims - #59007
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Looks good. core-js was only imported in runtime/polyfills.ts, so it can be dropped. Each shim only installs when the native API is missing, and for the ways the app calls them they behave like the native APIs: groups keep first-seen order and get the index, resolvers settle the promise, and Promise.try runs the callback synchronously and turns a throw into a rejection.
How I checked: the new polyfills.test.ts passes (3/3), and tsc -b in packages/app is clean. I couldn't test an older Safari end to end.
Public API: unchanged.
No concerns.
3 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
Drops the
core-jsdependency from the app.runtime/polyfills.tspulled it in for three APIs (Map.groupBy,Promise.withResolvers,Promise.try); it now has small local shims instead.How it works
Promise.try), so the desktop app never needs a polyfill.target: "esnext"and has no browserslist, and it is served to Safari.Map.groupByandPromise.withResolversneed Safari 17.4,Promise.tryneeds 18.2, and the app calls all three. So the polyfills can't be dropped outright.Risk
Web app on Safari older than 18.2 only. The shims follow the spec for the cases the app uses (no iterable-edge-case shimming beyond that). The desktop app is unaffected.
Proof
bun testforsrc/runtime/polyfills.test.ts(removes the natives, loads the shims, checks grouping order and index, resolver settling, synchronous run and throw isolation): 3 pass.bun typecheckinpackages/app: clean.bun script/lint-changed.ts origin/dev: 0 problems.