Remove unused Promise polyfill and promise dependency - #57777
Open
aravi365 wants to merge 1 commit into
Open
Conversation
Every supported JS engine ships a native Promise, but the polyfill branch in polyfillPromise.js is statically reachable, so Metro bundles the promise package into every app even though it is never used. Remove the dead branch, Libraries/Promise.js and the promise dependency. DEV unhandled-rejection tracking is unchanged. Fixes react#57702
|
@robhogan has imported this pull request. If you are a Meta employee, you can view this in D114354694. |
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.
Summary
Fixes #57702.
polyfillPromise.jsfalls back torequire('../Promise')whenHermesInternal.hasPromise()is falsy. That require is statically reachable, soMetro bundles the
promisepackage into every app even though Hermes apps neverexecute it. Removing the fallback drops the dependency and the dead code from the
bundle.
This removes:
polyfillPromise.jsLibraries/Promise.jspromisedependency and its flow-typed defDEV unhandled-rejection tracking is unchanged, it goes through
HermesInternal.enablePromiseRejectionTracker.promiseRejectionTrackingOptions.jsimported its options type from
promise/setimmediate/rejection-tracking, so thattype is now declared inline.
One thing worth a maintainer's call: the fallback also covered non-Hermes engines
(JSC, V8), which now get the engine's native
Promiseinstead. Both ship aspec-compliant implementation today, and the "Event loop not supported" comment
this code was written for no longer applies, but if you'd rather keep the polyfill
for those engines I can gate it instead of deleting it.
Changelog:
[GENERAL] [REMOVED] - Remove the unused Promise polyfill and the promise dependency
Test Plan
git grepfinds no remaining references toLibraries/Promiseorpromise/setimmediatein the repo.promiseline is removed frompackage.json.hasPromise()is true, soglobal.Promisewas already the one in use, and the DEV rejection tracker still gets the same
options object.
I have not run the full JS suite locally, relying on CI for
test_js, Flow and lint.