Upgrade to react-scripts 5 #249

dylnclrk · 2022-01-15T01:06:47Z

Problem

The CRA team released react-scripts 5.0.0 recently. Depfu opened #242 to address this, however, react-scripts is responsible for generating many of the files in this repo, so PR does the upgrade manually to capture those changes.

Solution

Create a new project using react-scrips 5.0.0, audit the differences in the generated files from those currently on main (which were generated by an older version of react-scripts). Copy over those changes and try to get rid of some of the cruft in this template (there wasn't much).

Change summary

Run create-react-app (with react-scripts 5.0.0) to generate a new, clean app-prototype
Copy over our changes like SCSS, linter configs, docker config, CCI config, etc.
Upgrade a handful of other packages that were lagging behind.

TODO

Fix docker build ($ docker build . -t spraygun-react-ts:latest, see comment)

Things to manually verify:

Steps to Verify

Create a spraygun project with this version of the TS template (by using spraygun's -p path/to/this/template flag)
See that it functions as expected (yarn test, lint, start, build, yarn serve -s build etc.)
Check that deploys to Heroku and runs on CCI as expected

New Dependencies

Additions:

web-vitals - comes with new CRA-generated apps, left it in.

🙈 See the package.json diff for all of the upgraded dependencies.

Screenshots

Appears to work locally:

dylnclrk · 2022-01-15T01:07:34Z

Still working on verifying that everything still works… Please let me know if you have suggestions of things to test.

dylnclrk · 2022-01-15T01:13:23Z

package.json

@@ -3,43 +3,41 @@
 "version": "0.1.0",
 "private": true,
 "dependencies": {
- "@types/node": "^16.11.11",
- "@types/react": "^17.0.37",
+ "@types/node": "^17.0.8",


Oops, I think this might need to track the current version of Node (16). I think the index.d.ts for @types/node should specify the Nodejs version.

dylnclrk · 2022-01-15T01:17:10Z

package.json

 "serve": "^13.0.2",
- "typescript": "^4.5.4"
+ "typescript": "^4.5.4",
+ "web-vitals": "^2.1.3"


🗒️ this is a new package, create-react-app adds it (and the required boilerplate) by default. With it you can report metrics like "first input delay", "time to first byte", etc from real users. I would use it on my next project, so I left it in. But maybe this warrants a separate PR? More info here: GooggleChrome/web-vitals

dylnclrk · 2022-01-15T01:34:03Z

package.json

 },
 "resolutions": {
- "@typescript-eslint/typescript-estree": "^5.6.0",


🗒️ I dropped this resolution @typescript-eslint/typescript-estree because I wasn't sure why it was in there, and getting rid of it didn't seem to have an effect.

dylnclrk · 2022-01-15T01:34:40Z

package.json

 },
 "resolutions": {
- "@typescript-eslint/typescript-estree": "^5.6.0",
- "caniuse-lite": "^1.0.30001272"
+ "caniuse-lite": "^1.0.30001299",


🗒️ Updated the caniuse-lite def to match what was installed from scratch

dylnclrk · 2022-01-15T01:35:54Z

package.json

- "@typescript-eslint/typescript-estree": "^5.6.0",
- "caniuse-lite": "^1.0.30001272"
+ "caniuse-lite": "^1.0.30001299",
+ "mini-css-extract-plugin": "2.4.7"


🤦 This is a temporary workaround for this issue: facebook/create-react-app#11930

dylnclrk · 2022-01-15T01:45:16Z

public/index.html

+ <meta
+ name="description"
+ content="Web site created using create-react-app"
+ />


❔ Should we remove this? Leave it in? change it to

Suggested change

<meta

name="description"

content="Web site created using create-react-app"

/>

<meta

name="description"

content="Web site created using spraygun"

/>

I think we should take it out. It would be weird for that to go into production accidentally, and then you find out when someone shares your app on social and that description pop up 😄

dylnclrk · 2022-01-15T01:46:15Z

public/robots.txt

@@ -0,0 +1,3 @@
+# https://www.robotstxt.org/robotstxt.html


🤔 Not sure if this was intentionally omitted before, but it was generated by create-react-app

dylnclrk · 2022-01-15T01:46:54Z

src/App.module.scss

@@ -34,7 +34,7 @@
 color: colors.$blue;
 }

-@keyframes App-logo-spin {
+@keyframes app-logo-spin {


🗒️ New stylelint rule wants keyframes to be lower-kebab-case.

dylnclrk · 2022-01-15T01:47:58Z

src/index.scss

@@ -1,6 +1,5 @@
 // index.scss is for CSS declarations that aren't specific to a component.
 // In practice, that means this file is used for two things:
-//


❔ New stylelint rule doesn't want blank comments like this, I sorta disagree, but followed the rule to keep things as vanilla as possible. Should we disable this?

I agree, this rule seems a bit much. I vote to disable the rule.

✅ disabled!

dylnclrk · 2022-01-15T01:50:44Z

src/index.tsx

+
+// If you want to start measuring performance in your app, pass a function
+// to log results (for example: reportWebVitals(console.log))
+// or send to an analytics endpoint. Learn more: https://bit.ly/CRA-vitals
+reportWebVitals();


🗒️ Here's the boilerplate for web-vitals. More info on the linked page.

dylnclrk · 2022-01-15T01:51:32Z

src/reportWebVitals.ts

@@ -0,0 +1,15 @@
+import { ReportHandler } from "web-vitals";
+
+const reportWebVitals = (onPerfEntry?: ReportHandler) => {


🤣 Would be funny if this code affected these metrics.

dylnclrk · 2022-01-15T01:52:13Z

src/setupTests.ts

@@ -2,4 +2,4 @@
 // allows you to do things like:
 // expect(element).toHaveTextContent(/react/i)
 // learn more: https://github.com/testing-library/jest-dom
-import "@testing-library/jest-dom/extend-expect";
+import "@testing-library/jest-dom";


🗒️ CRA generates this file with the shorter import (which matches the docs)

dylnclrk · 2022-01-15T01:53:17Z

src/theme/colors.scss

+@use "sass:color";
 $blue: #61dafb;
-$blue-darker: darken($blue, 30%);
-$blue-lighter: lighten($blue, 5%);
+$blue-darker: color.adjust($blue, $lightness: -30%);
+$blue-lighter: color.adjust($blue, $lightness: 5%);


🗒️ stylelint doesn't want you to use darken/lighten any more.

dylnclrk · 2022-01-15T01:55:09Z

.eslintrc.js

@@ -23,6 +23,7 @@ module.exports = {
 },
 extends: [
 "react-app",
+ "react-app/jest",


🗒️ CRA includes this extend now. I wonder if that includes plugin:jest/recommended, which makes the below extend a dupe.

dylnclrk · 2022-01-15T02:13:23Z

Oof, looks like the docker build needs to be fixed, it appears to be building a test file.

dylnclrk · 2022-01-19T00:39:31Z

Oof, looks like the docker build needs to be fixed, it appears to be building a test file.

Was having problems maintaining the dependencies/devDependencies split, facebook/create-react-app#11956, merging dependencies into one group (as is the default w/ CRA) for now.

mattbrictson · 2022-01-21T01:32:38Z

Create a spraygun project with this version of the TS template (? not sure how to do this with a branch)

@dylnclrk You can use the -p (local path) option like this:

$ npx spraygun -p path/to/spraygun-react-ts APP_NAME

dylnclrk · 2022-01-21T01:47:31Z

Create a spraygun project with this version of the TS template (? not sure how to do this with a branch)

@dylnclrk You can use the -p (local path) option like this:
$ npx spraygun -p path/to/spraygun-react-ts APP_NAME

Oh, sorry, I should have updated those instructions to get rid of that question. But yes, I tested it and even deployed the generated app to Heroku on Tuesday and everything seemed to work as expected.

mattbrictson

Looks great! Thanks for testing the Heroku deploy as well.

I tested locally with npx spraygun -p ... and it worked great.

I noticed a deprecation warning when starting the dev server, but that seems to be a known CRA issue: facebook/create-react-app#11860. So I don't think that should block this PR.

🚀

mattbrictson · 2022-01-21T01:46:11Z

src/index.scss

@@ -1,6 +1,5 @@
 // index.scss is for CSS declarations that aren't specific to a component.
 // In practice, that means this file is used for two things:
-//


I agree, this rule seems a bit much. I vote to disable the rule.

dylnclrk · 2022-02-02T01:25:40Z

I upgraded a couple dependencies to match main, checked everything again and this is good to go 🚀 merge at will…

mattbrictson

💥

dylnclrk added 3 commits January 14, 2022 16:34

Upgrade to react-scripts 5.0.0

78dab04

Update @types/node

7a62500

Update stylelint

327f2f0

dylnclrk requested a review from a team January 15, 2022 01:06

dylnclrk commented Jan 15, 2022

View reviewed changes

dylnclrk added 2 commits January 14, 2022 17:22

Add eslint as explicit dependency

aa0a3d1

Downgrade @types/node to match node version

eb310cf

dylnclrk force-pushed the update-to-react-scripts-5 branch from f59927a to eb310cf Compare January 15, 2022 01:33

dylnclrk commented Jan 15, 2022

View reviewed changes

dylnclrk mentioned this pull request Jan 15, 2022

Upgrade react-scripts: 4.0.3 → 5.0.0 (major) #242

Closed

dylnclrk added 4 commits January 18, 2022 16:40

Add all dependencies to one group

9a85a78

Get rid of potentially confusing description

89e043f

Merge branch 'main' into update-to-react-scripts-5

40af361

Resolve lockfile conflicts by re-generating

254ed9d

mattbrictson approved these changes Jan 21, 2022

View reviewed changes

dylnclrk added 3 commits February 1, 2022 17:11

Disable unreasonable stylelint rule

a10f741

Update deps

63436e6

Merge branch 'main' into update-to-react-scripts-5

131b5d1

mattbrictson approved these changes Feb 2, 2022

View reviewed changes

mattbrictson merged commit 6749b78 into main Feb 2, 2022

mattbrictson deleted the update-to-react-scripts-5 branch February 2, 2022 01:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to react-scripts 5 #249

Upgrade to react-scripts 5 #249

dylnclrk commented Jan 15, 2022 •

edited

Loading

dylnclrk commented Jan 15, 2022

dylnclrk Jan 15, 2022

dylnclrk Jan 15, 2022

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022

dylnclrk Jan 15, 2022

mattbrictson Jan 19, 2022

dylnclrk Jan 19, 2022

dylnclrk Jan 15, 2022

dylnclrk Jan 15, 2022

dylnclrk Jan 15, 2022 •

edited

Loading

mattbrictson Jan 21, 2022

dylnclrk Feb 2, 2022

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk commented Jan 15, 2022

dylnclrk commented Jan 19, 2022

mattbrictson commented Jan 21, 2022

dylnclrk commented Jan 21, 2022

mattbrictson left a comment

mattbrictson Jan 21, 2022

dylnclrk commented Feb 2, 2022

mattbrictson left a comment

		@@ -0,0 +1,15 @@
		import { ReportHandler } from "web-vitals";

		const reportWebVitals = (onPerfEntry?: ReportHandler) => {

Upgrade to react-scripts 5 #249

Upgrade to react-scripts 5 #249

Conversation

dylnclrk commented Jan 15, 2022 • edited Loading

Problem

Solution

Change summary

TODO

Steps to Verify

New Dependencies

Screenshots

dylnclrk commented Jan 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

dylnclrk Jan 15, 2022 • edited Loading

Choose a reason for hiding this comment

dylnclrk commented Jan 15, 2022

dylnclrk commented Jan 19, 2022

mattbrictson commented Jan 21, 2022

dylnclrk commented Jan 21, 2022

mattbrictson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylnclrk commented Feb 2, 2022

mattbrictson left a comment

Choose a reason for hiding this comment

dylnclrk commented Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading

dylnclrk Jan 15, 2022 •

edited

Loading