Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

[FRONTEND-191] Update to v4.0.0 #29

Merged
merged 129 commits into from
Nov 26, 2020
Merged

[FRONTEND-191] Update to v4.0.0 #29

merged 129 commits into from
Nov 26, 2020

Conversation

panbenson
Copy link

Update our react-scripts version to v4.0.0 🎉

  • Tested with league-web
  • just to keep track: there were a few minor conflicts in the following files:
    • package.json
    • packages/react-scripts/package.json
    • packages/react-scripts/config/webpack.config.js
    • azure-pipelines.yml

Reference to this detailed guide we have 😄 https://everlong.atlassian.net/wiki/spaces/TECH/pages/780567565/League+s+Create+React+App+React+Scripts+fork

MichaelDeBoey and others added 30 commits March 24, 2020 09:10
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Bumps [acorn](https://github.com/acornjs/acorn) from 6.4.0 to 6.4.1.
- [Release notes](https://github.com/acornjs/acorn/releases)
- [Commits](acornjs/acorn@6.4.0...6.4.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Passing an array with a single entry is not equivalent. This causes Webpack
to generate another wrapper module around the entry. This is just
unnecessary overhead and bytes.
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
MichaelDeBoey and others added 14 commits October 22, 2020 15:32
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
 - babel-plugin-named-asset-import@0.3.7
 - babel-preset-react-app@10.0.0
 - confusing-browser-globals@1.0.10
 - cra-template-typescript@1.1.0
 - cra-template@1.1.0
 - create-react-app@4.0.0
 - eslint-config-react-app@6.0.0
 - react-app-polyfill@2.0.0
 - react-dev-utils@11.0.0
 - react-error-overlay@6.0.8
 - react-scripts@4.0.0
@panbenson panbenson self-assigned this Nov 18, 2020
@panbenson
Copy link
Author

Update: Looks like theres quite a few changes needed to have this change in league-web. i'll get back to this some other time but will document what i find

  1. theres new linting stuff that breaks the build:prod and affects like a million files
    • i.e. react/prop-types, jsx-a11y/label-has-associated-control, react/destructuring-assignment, react/prop-types, etc.
  2. storybook needs to be fixed
    • Storyshot depends on require.requireActual which has removed from Jest 26 (though i haven't played with downgrading to jest 25 for our CRA fork)
    • i tried to updated to storybook 6.0 but quite a few other things break (something with snapshotWithOptions in initiStoryshots)
      • withA11y decorator is removed
      • argument for the addDecorator callback is now a react element instead of a function
  3. SVG imports to be updates (thanks for catching this @nicolechung)
    • i.e. all imports like import { ReactComponent as MoneyBag } from './assets/money-bag-dollar.svg'; needs to be import MoneyBag from './assets/money-bag-dollar.svg';

package.json Outdated
@@ -31,7 +31,7 @@
"get-port": "^5.1.1",
"globby": "^11.0.1",
"husky": "^4.3.0",
"jest": "26.6.0",
Copy link
Author

@panbenson panbenson Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jest 26 breaks storybook 5, so downgrading to jest 25. Updating storybook was way too much to include at once 😅

},
},
}),
// START: EVERLONG CHANGES
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to leave the linting out for now, especially since we have the pre-commit linting on league-web. This linting lints everything and runs on both yarn build and yarn start, which is a pain because it takes a really long time. Plus, no linting errors are reported currently when we run yarn build:prod

for reference: yarn build:prod

  • took 243s with react-scripts v4
  • took 374s with our current react-scripts

@@ -89,7 +89,8 @@
"webpack": "4.44.2",
"webpack-dev-server": "3.11.0",
"webpack-manifest-plugin": "2.2.0",
"workbox-webpack-plugin": "5.1.4"
"workbox-webpack-plugin": "5.1.4",
"typescript": "3.9.7"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is another hack which lets me compile instead of getting this error message:

Failed to load plugin '@typescript-eslint' declared in '.eslintrc » eslint-config-react-app#overrides[0]': Cannot find module 'typescript'

Copy link

@theinterned theinterned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes here LGTM – I'm about to test https://github.com/EverlongProject/league-web/pull/4702 now.

@theinterned theinterned changed the title Update to v4.0.0 [FRONTEND-191] Update to v4.0.0 Nov 19, 2020
Copy link

@foreverhomes foreverhomes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, nice!

@nicolechung
Copy link

error puppeteer@3.3.0: The engine "node" is incompatible with this module. Expected version ">=10.18.1". Got "10.16.3"
error Found incompatible module.

Did anyone else get this error on yarn ing?

@panbenson
Copy link
Author

error puppeteer@3.3.0: The engine "node" is incompatible with this module. Expected version ">=10.18.1". Got "10.16.3"
error Found incompatible module.

Did anyone else get this error on yarn ing?

I needed to do nvm use 10.18.1 to get the packages to install

@nicolechung
Copy link

I needed to do nvm use 10.18.1 to get the packages to install

This worked for me! Although I think it might present a problem as we are forcing people to use 10.16 on League-web, we might have to look into updating the env/direnv settings on league-web

@panbenson panbenson merged commit 470c8f6 into main Nov 26, 2020
@panbenson panbenson deleted the merge-v4.0.0 branch November 26, 2020 16:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.