Fix polyfills working incorrectly on Edge #2743
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2743 +/- ##
=======================================
Coverage 45.95% 45.95%
=======================================
Files 248 248
Lines 5273 5273
Branches 1222 1222
=======================================
Hits 2423 2423
Misses 2850 2850
Continue to review full report at Codecov.
|
|
Deployment preview for |
chrisgzf
changed the title
website/src/entry/App.tsx
Outdated
| @@ -7,6 +7,11 @@ import { hot } from 'react-hot-loader/root'; | |||
| import { BrowserRouter as Router } from 'react-router-dom'; | |||
| import { Provider } from 'react-redux'; | |||
| import { PersistGate } from 'redux-persist/integration/react'; | |||
| import 'core-js/es/promise/finally'; | |||
There was a problem hiding this comment.
Is this order correct? I think the polyfill should go as early as possible, since you are importing for side-effect here. Technically from the stacktrace it looks like the error would only manifest on mount, but for safety polyfills should be loaded as early as possible since the other modules can also have side effect / call the polyfilled methods during init
There was a problem hiding this comment.
Also another nit - comments usually go above the line they are commenting on, not below
|
Also good job sleuthing out this solution! |
|
Also I think Packtracker has the same issue as the other GitHub apps in that it has issue with PRs from forks, which is rather annoying :( Thanks for manually validating the size issue |
|
Wow great work @chrisgzf! |
|
Sorry, one last issue - the polyfill added in the wrong file - App.tsx is not the entry point. It should be |
|
@ZhangYiJiang fixed in 09ca440! |
Context
Users who use the non-Chromium Edge were reporting that they were seeing a white-screen of death when loading NUSMods, caused by:
Which is caused by:
PersistGatefrom one of our deps,redux-persistusesPromise.prototype.finallywhich somehow isn't supported on Edge.But no, the story doesn't end here.
Promise.prototype.finallyactually IS SUPPORTED by Edge. Except that there is some oddity with our polyfill librarycore-jsor with Edge that causes it to fail Promise feature detection, and Promise is entirely polyfilled incorrectly bycore-json Edge, causing the native implementation offinallyto be gone.To resolve this, we polyfill
finally.Implementation
Tried 3 methods
require.resolve('redux-persist/integration/react')to the list of files transpiled by Babel. This should make a lot of sense, but somehowcore-jshas issues with adding polyfills to deps. ❌useBuiltIns: usagetouseBuiltIns: entryin Babel's config, so that it adds every single stable polyfill fromcore-jsto every file. Works great, but greatly increases the bundle size (+13 KB), see WIP Promise.prototype.finally polyfill #2741. ✅Tested on Edge 44.17763.1.0 (which is from donkey years ago).
Other Information
Congrats to the 70 people who use Edge! 🥴 🤢 🤮 You can now use NUSMods again!