[MR-886] Upgrade react-scripts to v5 #914

haworku · 2022-04-12T15:19:23Z

Summary

Upgrade react-scripts (used to build create react app) to version 5. This is built on webpack 5

Notes:

Several changes needed to support uswds SCSS import properly within webpack 5 paradigm.
Major issue complicating this upgrade was Storybook and react-router conflicts. To resolve this, without also doing the react-router upgrade in this PR, made the choice to go up to a storybook prerelease version that resolves the types bundling issue with react-router. More about this bug - Storybook v6.4 is breaking users using react-router v5 storybookjs/storybook#16837 .
Had to make changes in app-api as well. webpack5 in app-web started to throw errors with prisma dependencies (were also packaged with webpack) unhappy. In order to get it working again added a single layer that has the prisma packages and forced webpack to not pack up prisma. That way the resolutions for prisma go to the lambda layer, which have the appropriate query engine builds.
Towards the end of this work, we ran into strange errors from the cypress pa11y node_modules. Commented out the cy.pa11y calls for now to get this work through.

Known compile warnings with this major version upgrade:

Failed to parse source maps for dependencies. Known issue After Upgrading to CRA 5.0 getting a lot of: Failed to parse source map from facebook/create-react-app#11767
Invalid dependencies (due to using relative paths) in sass loaders caused by uswds related changes. Ideally need to use absolute paths, but could not get that compiling. This may be uswds bug and/or it could also just be a limitation of the monorepo setup. Need to explore more later.

Related issues

https://qmacbis.atlassian.net/browse/MR-886

Screenshots

Test cases covered

No new tests added but app-web tests that reference the browser location were modified to use this pattern.

QA guidance

Manual QA of areas of the app not covered well by tests (variations on submissions, unlock resubmit workflow multiple times, logging in and out with IDM).

Bumps [react-scripts](https://github.com/facebook/create-react-app/tree/HEAD/packages/react-scripts) from 4.0.3 to 5.0.0. - [Release notes](https://github.com/facebook/create-react-app/releases) - [Changelog](https://github.com/facebook/create-react-app/blob/main/CHANGELOG.md) - [Commits](https://github.com/facebook/create-react-app/commits/react-scripts@5.0.0/packages/react-scripts) --- updated-dependencies: - dependency-name: react-scripts dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

… webpack 5 friendly export artifacts

…le names in scss

…cripts 4.x

… react-scripts 4.x" This reverts commit 99142f0.

…eps from react-scripts 4.x"" This reverts commit e322691.

…ter to avoid react-router conflict

…et app compiling

…tly referencing Submission2

…mpile error

This reverts commit 0aab907.

…not found

…nfra deploying

haworku · 2022-04-12T15:20:52Z

services/app-web/package.json

@@ -89,28 +89,29 @@
        "dayjs": "^1.10.5",
        "formik": "^2.2.9",
        "graphql": "^16.2.0",
+        "history": "^5.3.0",


new temp dependency - added until we complete react-router upgrade to address types clashing. After upgrade it will be removed again. reason for this is described more in programming notes.

haworku · 2022-04-12T15:21:06Z

services/app-web/package.json

        "react": "^17.0.2",
-        "react-aria-modal": "^4.0.0",


this was an unused dependency

haworku · 2022-04-12T15:21:36Z

services/app-web/package.json

+        "@storybook/addon-a11y": "^6.5.0-alpha.61",
+        "@storybook/addon-essentials": "^6.5.0-alpha.61",
+        "@storybook/addon-links": "^6.5.0-alpha.61",
+        "@storybook/builder-webpack5": "^6.5.0-alpha.61",


new temp dependencies - only on prerelease until we complete react-router upgrade. Then we can go back down. reasoning described more in programming notes.

macrael

This looks great. You've got some Prisma lambda layer changes hanging out in this PR that doesn't seem related to the react-scripts upgrade, IDK where they came from or if they are right.

macrael · 2022-04-21T16:47:59Z

.github/workflows/deploy-app-to-env.yml

        run: |
          tar -C ./services/app-api/lambda-layers-prisma-client-migration -xf ./services/app-api/lambda-layers-prisma-client-migration/nodejs.tar.gz
          rm -rf ./services/app-api/lambda-layers-prisma-client-migration/nodejs.tar.gz

+      - uses: actions/download-artifact@v3


why is this change in the react-scripts PR?

Looks like this came in with adf1882. @mojotalantikite @macrael does that commit look alright to yall just sanity check because we had a bit of funky stuff with app-web merge at least want to be sure.

So yeah, with the webpack5 changes there was a change in behavior that stopped prisma from working again. Basically it was copying in parts of the prisma client, but not any of the engine things that it needs.

In order to get it working again I added a single layer that has the prisma packages and forced webpack to not pack up prisma. That way the resolutions for prisma go to the lambda layer, which have the appropriate query engine builds.

macrael · 2022-04-21T16:50:21Z

services/app-web/src/pages/Auth/Auth.test.tsx

-            })
+            let testLocation: Location
+
+            renderWithProviders(


Every time I see this function it makes me happy. @haworku this testing function is brilliant, giving us usable defaults and making it easy to customize things per-test.

haworku · 2022-04-21T17:43:25Z

6b79287
206616d
dd097a5
Are all fixup commits from a funky merges with main.

haworku · 2022-04-21T18:48:14Z

cypress is failing with errors I haven't seen before 🔍 . Its also happening locally and pointing to where we import pa11y in the cypress plugins file. Does any of this sound familiar @macrael

haworku · 2022-04-22T14:29:26Z

Unfortunately had to comment out our accessibility automation cy.pa11y calls due to errors seemingly from inside pa11y node_modules to get this through.

Will make a ticket before end of day to capture this issue and put at top of pilot backlog. It might just be we need to probably isolate the tests folder (and cypress) in its own service in packages, instead of where it is sitting now. The way things are set up now, with deps in the root package.json, and a nested tsconfig, could be off.

macrael

Still looks good

mojotalantikite · 2022-04-22T16:56:12Z

I say ship it!

But reminder that we'll have to swap the deploy-app-to-env.yml tag back to main in a follow on PR since I made some CI changes.

dependabot bot and others added 25 commits April 11, 2022 10:13

Upgrade uswds even though its out of sync with react-uswds to get the…

bb1985a

… webpack 5 friendly export artifacts

Remove tilde from sass imports, since its deprecated, and use full fi…

3316e54

…le names in scss

Upgrade storybook react dependencies, remove pinned deps from react-s…

5106595

…cripts 4.x

Remove unused dependenct react-aria-modal

85cdbe3

Fix up more issues related to scss compiling

864852e

Revert "Upgrade storybook react dependencies, remove pinned deps from…

39ce622

… react-scripts 4.x" This reverts commit 99142f0.

Fixing uswds asset path reduces errors from 227 to 162

2cb577e

Revert "Revert "Upgrade storybook react dependencies, remove pinned d…

c052434

…eps from react-scripts 4.x"" This reverts commit e322691.

Address all compile errors, on to warnings

90be8a4

Upgrade sass

9f941b3

Manually resolve react router deps

7635509

Remove node modules and regenerate yarn.lock

79e9da8

Upgrade to storybook 6 and add cra5 builder

cca0caa

setup router compatibility package

0aab907

Upgrade to next storybook prerelase, which pre-bundles @storybook/rou…

ab2dbcb

…ter to avoid react-router conflict

Temp commit - comment out all reference to history to see if we can g…

31cd211

…et app compiling

Address Typescript warnings in ChangeHistory.test.tsx due to incorrec…

b8b2f85

…tly referencing Submission2

Apparently we need to import Buffer now to use it in app-web - fix co…

542aa95

…mpile error

Revert "setup router compatibility package"

ea800da

This reverts commit 0aab907.

Add serverless-webpack to uploads to address CORS error from webpack …

77b80b4

…not found

Remove unnecessary history dep

eac7aae

Merge remote-tracking branch 'origin/main' into hw-upgrade-react-scripts

a0946d6

app-web tests passing

ebf9560

Add the other webpack dep from app-api to uploads, see if this gets i…

c2b10e9

…nfra deploying

haworku commented Apr 12, 2022

View reviewed changes

services/app-web/package.json

"react": "^17.0.2",

"react-aria-modal": "^4.0.0",

Copy link

Contributor Author

haworku Apr 12, 2022

Choose a reason for hiding this comment

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

this was an unused dependency

haworku commented Apr 12, 2022

View reviewed changes

Upgrade ts-jest and jest to address app-api errors

64b021d

github-actions bot requested a deployment to review-apps April 12, 2022 15:27 In progress

github-actions bot requested a deployment to review-apps April 18, 2022 20:03 In progress

Have serverless force exclude prisma. In a layer.

428cf21

github-actions bot requested a deployment to review-apps April 18, 2022 20:17 In progress

Fix cleaning

5428080

github-actions bot requested a deployment to review-apps April 18, 2022 20:45 In progress

mojotalantikite added 3 commits April 18, 2022 17:02

Remove debug

953a964

Merge branch 'main' into hw-upgrade-react-scripts

e6004db

Merge branch 'main' into hw-upgrade-react-scripts

b5fdf07

github-actions bot requested a deployment to review-apps April 18, 2022 21:38 In progress

Sync resolutions. Make sure types are in devdep

a855986

github-actions bot requested a deployment to review-apps April 18, 2022 21:44 In progress

Merge remote-tracking branch 'origin/main' into hw-upgrade-react-scripts

caa137d

macrael approved these changes Apr 21, 2022

View reviewed changes

haworku added 3 commits April 21, 2022 12:24

app-web lint passing

6b79287

app-api linting passing

206616d

Fix app-web ReviewSubmit tests again

dd097a5

github-actions bot deployed to review-apps April 21, 2022 17:38 View deployment

Comment out problematic cy.pa11y calls to unblock this PR merge.

7766052

github-actions bot deployed to review-apps April 22, 2022 13:43 View deployment

haworku marked this pull request as ready for review April 22, 2022 14:25

haworku requested a review from rswerve April 22, 2022 14:25

haworku requested review from macrael and mojotalantikite April 22, 2022 15:20

macrael approved these changes Apr 22, 2022

View reviewed changes

haworku merged commit 70c010d into main Apr 22, 2022

haworku deleted the hw-upgrade-react-scripts branch April 22, 2022 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MR-886] Upgrade react-scripts to v5 #914

[MR-886] Upgrade react-scripts to v5 #914

haworku commented Apr 12, 2022 •

edited

Loading

haworku Apr 12, 2022 •

edited

Loading

haworku Apr 12, 2022

haworku Apr 12, 2022

macrael left a comment

macrael Apr 21, 2022

haworku Apr 21, 2022 •

edited

Loading

mojotalantikite Apr 21, 2022 •

edited

Loading

macrael Apr 21, 2022

haworku commented Apr 21, 2022

haworku commented Apr 21, 2022 •

edited

Loading

haworku commented Apr 22, 2022 •

edited

Loading

macrael left a comment

mojotalantikite commented Apr 22, 2022

[MR-886] Upgrade react-scripts to v5 #914

[MR-886] Upgrade react-scripts to v5 #914

Conversation

haworku commented Apr 12, 2022 • edited Loading

Summary

Related issues

Screenshots

Test cases covered

QA guidance

haworku Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

haworku Apr 12, 2022

Choose a reason for hiding this comment

haworku Apr 12, 2022

Choose a reason for hiding this comment

macrael left a comment

Choose a reason for hiding this comment

macrael Apr 21, 2022

Choose a reason for hiding this comment

haworku Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

mojotalantikite Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

macrael Apr 21, 2022

Choose a reason for hiding this comment

haworku commented Apr 21, 2022

haworku commented Apr 21, 2022 • edited Loading

haworku commented Apr 22, 2022 • edited Loading

macrael left a comment

Choose a reason for hiding this comment

mojotalantikite commented Apr 22, 2022

haworku commented Apr 12, 2022 •

edited

Loading

haworku Apr 12, 2022 •

edited

Loading

haworku Apr 21, 2022 •

edited

Loading

mojotalantikite Apr 21, 2022 •

edited

Loading

haworku commented Apr 21, 2022 •

edited

Loading

haworku commented Apr 22, 2022 •

edited

Loading