Upgrade react-scripts to latest version (2.1.3) #99
Conversation
This file contains 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
|
@toolness I'm currently using node v8.9.1 on my dev machine :) |
toolness
changed the title
|
@sraby Ok I think this is good to go! I kicked the tires on IE11, Edge, Chrome, Firefox, and iPhone Safari. |
|
Er, actually hold on a sec @sraby... @romeboards is investigating whether the current WoW supports IE9 and IE10. If it does, then I will have to add the react polyfill thing for it (and we will have to test those browsers going forward, which is hard even for me on Windows b/c I only have IE11 here, so I guess I will have to use BrowserStack). |
|
Ok, Dan says we're only supporting IE11 so I think this is indeed good to go. |
toolness
added a commit
that referenced
this pull request
Jul 24, 2020
Looking at Rollbar errors for the latest front-end version made me realize that there were arrow functions in our JS bundles. Upon further inspection, I realized our `browserslist` specifies `not ie <= 11`, yet our `index.js` imports both `react-app-polyfill/ie11` and `babel-polyfill`. These contradictory lines were actually all introduced in the same PR, #99, which is very odd. I think the reason we might not have seen this strange behavior when writing that PR is because apparently [changing `browserslist` requires deleting `node_modules/.cache`][1]. So it's possible that during development, we never truly saw the results of our actions! (Needless to say, I spent a lot of time in confusion after making this PR's simple code change, until I discovered the root cause.) This means that even Netlify won't reflect changes to `browserslist` by default (it caches all of `node_modules` between builds), so I've modified its configuration to clear `node_modules/.cache` before building the site too. [1]: facebook/create-react-app#8197 (comment)
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.
Note: This is a PR against #95, notmaster! I will re-target it tomasteronce #95 is merged.This is an attempt to upgrade react-scripts to the latest version--partially just to stay up-to-date, but also because 2.1.0 and higher support Typescript out of the box, which could be useful in sharing code between this app and the tenants app, specifically for #53.
To do
require.ensure()(also a breaking change in 2).import()follows new semantics in 2 (not an issue, we don't use dynamic import)jsdomin 2 but we don't have any tests in it yet).PropTypesin weird ways (another breaking change in 2).