-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Framework: Move @babel/polyfill import to webpack config #26094
Conversation
@ockham I tried to give it a run on the same setup I used for the other issue and unfortunately, it looks like the init is broken on the UC Mini browser in prod again :(
Will try and see if I can get more info. |
My gut says this is related to #26120. I'm trying a rebase to see if it fixes it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut says this is related to #26120. I'm trying a rebase to see if it fixes it.
Suspicion confirmed. After rebasing this on master
, I'm able to boot Calypso on Android 4.4 / UC Mini and I noticed no regressions.
Awesome, thanks so much @jblz! |
@ockham This PR increased the bundle size and reverted the improvements achieved in #25961. It's because Is there a way to fix this? |
Ouch. Sorry 'bout that, I should've checked ICFY. I guess we can revert for now. My rationale for filing this PR was #24788, which wouldn't work without this change -- which probably means that |
Here's an idea for a simple fix: create a file import '@babel/polyfill'; and then use it in the entrypoint array: entry: {
build: [ './client/boot/babel-polyfill', './client/boot/app' ]
} That simple indirection will make the I didn't test it, so it might not work. Also, it might break Lerna again if the overeager |
Thanks for the suggestion! Unfortunately, that gives the same error as before 🙁 |
(In combination with #24788 that is -- meaning we might as well revert.) |
)" This reverts commit ed6beaa.
)" This reverts commit ed6beaa.
)" (#26192) This reverts commit ed6beaa. [Rationale](#26094 (comment)): > This PR [increased](http://iscalypsofastyet.com/push/ed6beaa5e0b54f6ddbe11360c8ad8a572b83a32f/e3642614723d88c353e5d768969d8f877d7472c4) the bundle size and [reverted](http://iscalypsofastyet.com/push/acef9fa493cc06a9fefece7ff00759997af8ca8a/03502467b044464e191c9746c510c6a10a7930e0) the improvements achieved in #25961. > > It's because `babel-preset-env` used to transform the `@babel/polyfill` import into a list that excludes the polyfills that are not needed on the target browser list. In the new location of `@babel/polyfill`, the plugin doesn't get the opportunity to do that transform and all polyfills are bundled again. I filed #26094 in the first place to get #24788 to work. Fortunately, @jsnajdr found [another way](#24788 (comment)).
For #24788 (which otherwise breaks). Usage in
webpack.config.js
is documented in official Babel docs.Testing Instructions
Verify that functionality that isn't present in a browser but provided by the polyfill still works.
In particular, follow testing instructions found in #25419 to make sure we're not introducing a regression here. @jblz I don't have Android Studio installed on my computer, can I ask you to give this a test?