[DR-3123] Migrate to vite #1475

samanehsan · 2023-08-09T21:58:14Z

This PR updates the UI to use Vite instead of create react app (react-scripts is one of the dependencies, which has not been maintained for over a year and contains a security vulnerability: https://broadworkbench.atlassian.net/browse/DR-3123).

Some things to note:

Vite has a built-in hot reloader
The route proxying is defined in vite.config.js (so setupProxy is no longer needed)
When running locally, the url needs to use "localhost" instead of "127.0.0.1" in order for log in to work

I also updated Cypress from version 9 -> 12, which supports the latest version of Vite. This involved:

Replacing cypress.json with cypress.config.ts
Removing the cypress/plugins/index.js file
Adding a cypress/support/component-index.html file (where the component test html gets rendered)
Updating the syntax in the e2e tests (e.g. cy.route is now cy.intercept) and some of the unit/component tests

samanehsan · 2023-08-15T16:36:08Z

The test datasets/snapshots used by the e2e tests exist but for some reason are not accessible to the test user (I can't access them with my dev account either). I created new ones in int-6 as a temporary work-around so I could run/fix the e2e tests locally, so they should pass once we can sort out the issue with int-4 and int-5!

samanehsan · 2023-08-16T16:56:21Z

The SonarCloud failure can be resolved by generating a hash for our fontawesome kit "https://kit.fontawesome.com/316fe749fb.js", but I get a 403 error when I try to do that (I used this website: https://www.srihash.org/). Does anyone have access? Or should I create a new custom kit?

okotsopoulos · 2023-08-16T19:55:27Z

@samanehsan that FontAwesome question rings a bell, I'm not sure if this is possible :(

From @snf2ye in February '23:

Fontawesome supports the use an SRI hash with versions <5. We are using a fontawesome kit v6. Fontawesome made a product choice to no longer support SRI hashes (source).

https://broadworkbench.atlassian.net/browse/DR-2872

samanehsan · 2023-08-17T18:00:45Z

Thanks @okotsopoulos! That's helpful context! I'll mark that script as excluded

samanehsan · 2023-08-21T14:28:52Z

I think the e2e tests aren't running because it waits on localhost:3000 but the vite dev server is using 127.0.0.1:3000. I'll see if there's a way to change the server to use localhost.

s-rubenstein

I'm noticing a lot more casting than was there before - is this a result of the libraries changing? Casting like that can sometimes be a code smell, so I am curious if you know where its coming from.

src/components/common/TextContent.test.tsx

s-rubenstein · 2023-08-21T14:47:27Z

src/components/common/WithoutStylesMarkdownContent.test.tsx

-    cy.get('div#__cy_root > div > p').should('contain.text', 'A bad day');
-    cy.get('div#__cy_root > div > p').children().should('have.length', 0);
+    cy.root().should('have.length', 1);
+    cy.root().get('p').should('contain.text', 'A bad day');


Just as a note, do you think this sort of test could maybe be simplified by doing a findByText on the page? That is what Terra-UI does

Ah oops I missed this comment! It turns out findByText requires another dependency so I'm inclined to leave it out for now if that's okay

s-rubenstein · 2023-08-21T14:48:54Z

src/constants/index.ts

@@ -1,5 +1,4 @@
 import { CloudPlatform } from 'generated/tdr';
-import _ from 'lodash';


I'm surprised you can remove the import from lodash without any other changes to the file. Do you have any linting for unused imports?

Good point! Our eslint rules don't include unused imports so I created a ticket for that: https://broadworkbench.atlassian.net/browse/DR-3176

s-rubenstein · 2023-08-21T14:51:18Z

src/reducers/snapshot.ts

        // Don't reset state if the only change was query parameters
        const path = action.payload?.location?.pathname;
        if (path === state.lastPath) {
          return immutable(state, {
-            lastPath: { $set: path },
-          });
+            lastPath: { $set: path as string },


I'm surprised you have to do the casts here. Does typescript not type interpolate correctly for these?

Yeah, weirdly there were a couple of places where the type couldn't be inferred and without making it explicit the server wouldn't run

Ah, in this particular case it might be that it thinks path is string | undefined given the ?s. Would you file a ticket to look into the specific casts? My guess is that they are resolvable using typescript but there may be some code structure changes needed.

Here's the ticket! https://broadworkbench.atlassian.net/browse/DR-3177

- The gcloud libraries are not pinned to a version so we cannot use an integrity hash here - Fontawesome no longer supports SRI hashes

Also reenable linting on server start

Work around wait-on 404 error Originally from: vitejs/vite#9520 (comment)

cypress · 2023-08-22T16:23:17Z

Passing run #2946 ↗︎

0	25	0	0	0

Details:

Merge `b73807f` into `7c334b7`...
Project: jade-data-repo-ui	Commit: `cd31fd1ca1 ℹ️`
Status: Passed	Duration: 08:52 💡
Started: Aug 30, 2023 2:07 PM	Ended: Aug 30, 2023 2:16 PM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

Fixes build issue with vite

samanehsan · 2023-08-23T16:05:02Z

src/store/middleware.ts

@@ -7,7 +7,7 @@ export const sagaMiddleware = createSagaMiddleware();
 const middleware = [sagaMiddleware];

 /* istanbul ignore next */
-if (process.env.NODE_ENV === 'development') {
+if (import.meta.env.DEV) {


This env variable is provided by Vite by default: https://vitejs.dev/guide/env-and-mode.html#env-variables

vite builds production by default

sonarqubecloud · 2023-08-23T18:38:07Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
8.5% Duplication

samanehsan · 2023-08-23T23:09:45Z

The "process not defined" issue causing the e2e tests to fail was due to to the version of jsondiffpatch we were using. Updating to the latest version fixed the issue! 🎉

s-rubenstein

LGTM

s-rubenstein · 2023-08-29T17:57:42Z

Dockerfile.direct

 # Copy the local code
 COPY . /
 # Copy the generated code
 COPY --from=codegen /local /
 # Run the build
 RUN export DISABLE_ESLINT_PLUGIN=true && \
  npm ci && \
-  npm run build-no-code-gen --production


Is this flag removed?

The --production flag isn't needed because vite build uses production by default

petesantos · 2023-08-29T19:03:38Z

package.json

@@ -24,6 +24,10 @@
  "dependencies": {
    "@babel/runtime": "^7.21.5",
    "@codemirror/lang-javascript": "^6.1.5",
+    "@cypress/vite-dev-server": "^5.0.5",


I believe there are some deps that can be moved to devDependencies. This isn't a big deal, but more for package.json hygiene

Feel free to do this in a follow up PR

petesantos · 2023-08-29T19:04:04Z

package.json

    "classlist-polyfill": "^1.2.0",
    "clsx": "^1.1.0",
    "connected-react-router": "^6.9.3",
-    "cypress": "^9.7.0",
+    "cypress": "^12.17.3",


This can be moved to devDependencies

petesantos · 2023-08-29T19:04:51Z

package.json

@@ -97,25 +98,27 @@
    "tss-react": "^4.8.4",
    "typescript": "^5.0.4",
    "validator": "^13.9.0",
+    "vite": "^4.4.9",


I believe vite can be moved to devDepenendcies

sonarqubecloud · 2023-08-30T13:54:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
5.7% Duplication

samanehsan mentioned this pull request Aug 9, 2023

[DR-3123] Replace react-scripts test server with vite #1468

Closed

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch from d2883a4 to 4384ecb Compare August 15, 2023 15:05

samanehsan marked this pull request as ready for review August 15, 2023 16:29

samanehsan requested review from snf2ye, nmalfroy and okotsopoulos as code owners August 15, 2023 16:30

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch from 9e599f1 to d5f8f36 Compare August 16, 2023 16:48

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch from d5f8f36 to 678f95c Compare August 17, 2023 18:04

s-rubenstein reviewed Aug 21, 2023

View reviewed changes

samanehsan added 14 commits August 21, 2023 15:59

Install vite related libraries and uninstall react-scripts

33a96ce

Update package.json scripts

2c7f971

Create vite config

d067654

Fix index.html

e4c5fa3

Get UI to run locally

327b301

Fix eslint

aabc6a6

Fix cypress tests

6833d19

Upgrade to Cypress 12 and fix component tests

0e2b3f1

Fix cypress e2e tests

6d6d897

Fix middleware

a6cae6a

Remove commented code

263d8ed

Ignore sonarcloud errors

081d451

- The gcloud libraries are not pinned to a version so we cannot use an integrity hash here - Fontawesome no longer supports SRI hashes

Clean up unused dependencies

c3cf84d

Use localhost instead of 127.0.0.1

9bf60d8

Also reenable linting on server start

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch from ecb4cbe to fcbb24e Compare August 21, 2023 20:00

Fix e2e test wait-on local server

64e2ad6

Work around wait-on 404 error Originally from: vitejs/vite#9520 (comment)

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch 2 times, most recently from 80fb251 to 7040f9f Compare August 22, 2023 14:52

Try replacing wait-on

92c71a7

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch 4 times, most recently from 7d41e77 to 2d265f2 Compare August 22, 2023 16:10

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch 5 times, most recently from de01eeb to 6b514bc Compare August 23, 2023 03:55

Update to from node 14->16

5b487a0

Fixes build issue with vite

samanehsan force-pushed the se/DR-3123-migrate-to-vite branch from 6b514bc to 5b487a0 Compare August 23, 2023 03:56

samanehsan commented Aug 23, 2023

View reviewed changes

Remove unnecessary --production flag

d9e1f6c

vite builds production by default

samanehsan added 3 commits August 23, 2023 16:52

Use default vite react plugin config

c7de517

Try defining process

29a696f

Try updating jsondiffpatch version

03c456e

samanehsan added 3 commits August 24, 2023 14:00

Remove unused packages

997ca29

Update .node-version

a50a81f

Fix cypress.config.ts

9c1c3b7

s-rubenstein approved these changes Aug 29, 2023

View reviewed changes

petesantos approved these changes Aug 29, 2023

View reviewed changes

Package clean-up

b73807f

samanehsan merged commit 5157015 into develop Aug 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DR-3123] Migrate to vite #1475

[DR-3123] Migrate to vite #1475

samanehsan commented Aug 9, 2023 •

edited

Loading

samanehsan commented Aug 15, 2023

samanehsan commented Aug 16, 2023

okotsopoulos commented Aug 16, 2023

samanehsan commented Aug 17, 2023

samanehsan commented Aug 21, 2023

s-rubenstein left a comment

s-rubenstein Aug 21, 2023

samanehsan Aug 24, 2023

s-rubenstein Aug 21, 2023

samanehsan Aug 24, 2023

s-rubenstein Aug 21, 2023

samanehsan Aug 21, 2023

s-rubenstein Aug 21, 2023

samanehsan Aug 24, 2023

cypress bot commented Aug 22, 2023 •

edited

Loading

samanehsan Aug 23, 2023

sonarqubecloud bot commented Aug 23, 2023

samanehsan commented Aug 23, 2023

s-rubenstein left a comment

s-rubenstein Aug 29, 2023

samanehsan Aug 29, 2023 •

edited

Loading

petesantos Aug 29, 2023

petesantos Aug 29, 2023

petesantos Aug 29, 2023

sonarqubecloud bot commented Aug 30, 2023

		@@ -1,5 +1,4 @@
		import { CloudPlatform } from 'generated/tdr';
		import _ from 'lodash';

[DR-3123] Migrate to vite #1475

[DR-3123] Migrate to vite #1475

Conversation

samanehsan commented Aug 9, 2023 • edited Loading

samanehsan commented Aug 15, 2023

samanehsan commented Aug 16, 2023

okotsopoulos commented Aug 16, 2023

samanehsan commented Aug 17, 2023

samanehsan commented Aug 21, 2023

s-rubenstein left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Aug 22, 2023 • edited Loading

Passing run #2946 ↗︎

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 23, 2023

samanehsan commented Aug 23, 2023

s-rubenstein left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samanehsan Aug 29, 2023 • 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

sonarqubecloud bot commented Aug 30, 2023

samanehsan commented Aug 9, 2023 •

edited

Loading

cypress bot commented Aug 22, 2023 •

edited

Loading

samanehsan Aug 29, 2023 •

edited

Loading