[VTAdmin] Migrate to Vite

[notfelineit]

Description

This PR addresses this issue and migrates VTAdmin web from create-react-app to vite. Some of the larger changes include:

• Switching to vitest for all tests
  • Removes jest since vitest replaces it
  • Removes repetitive beforeAll/afterEach/afterAll since vitest has a global config file
• Renaming REACT_APP_* env variables to VITE_
• Changing process.env to import.meta.env
• Changing protobuf output from commonjs to es6 to better work with vite's defaults
• Configures eslint with vite and fixes some lint errors
• Fixes some prettier errors

Manual validations

• Ran ./101_initial_cluster.sh and VTAdmin web loads fine 👍
• Ran npm run build locally and verified build runs locally
• Ran npm run local and verified env vars passed are respected

Related Issue(s)

• Issue: [vtadmin-web] Move off of create-react-app #12280
• Web docs PR: [v17] Update from create-react-app to vite website#1437

Checklist

• "Backport to:" labels have been added if this change should be back-ported No need for backport
• Tests were added or are not required Tests have been modified to use vitest
• Did the new or modified tests pass consistently locally and on the CI Yes
• Documentation was added or is not required Docs are here

Deployment Notes

N/A

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.
• If a test is added or modified, there should be a documentation on top of the test to explain what the expected behavior is what the test does.

If a new flag is being introduced:

• Is it really necessary to add this flag?
• Flag names should be clear and intuitive (as far as possible)
• Help text should be descriptive.
• Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow should be required, the maintainer team should be notified.

Bug fixes

• There should be at least one unit or end-to-end test.
• The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

• There should be some code comments as to why things are implemented the way they are.

New/Existing features

• Should be documented, either by modifying the existing documentation or creating new documentation.
• New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• vtctl command output order should be stable and awk-able.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from VTop, if used there.

[frouioui]

Left several comments about license header missing, but there are a few other places where the headers are missing

[frouioui]

I am not an expert in VTAdmin so I'll let @ajm188 stamp this. But everything outside of VTAdmin and the few comments I left looks good to me 🙏🏻

[ajm188]

LGTM! Just a couple small things, feel free to fix-and-merge!

[ajm188]

oh noooo lol

i completely get the name but boy that's gonna be confusing hahaha

[notfelineit]

😅 yeah it will be, we could add (NOT VITEST!!) afterwards lol

[dbussink]

I think we'll have to disable this workflow for this PR: https://github.com/vitessio/vitess/actions/runs/4620726123/workflow?pr=12831 since the PR itself modifies the make vtadmin_web_proto_types script.

That shouldn't be needed if what the new version creates is also committed?

[notfelineit]

@dbussink we wouldn't need to disable it if the workflow uses this branch's generate-proto-types.sh file 🤔 wasn't sure if that was the case or not.

The CI step is still failing. The protos generated are still the latest though:

➜  vitess git:(frances/migrate-from-create-react-app) make vtadmin_web_proto_types

added 245 packages, removed 1112 packages, changed 518 packages, and audited 1375 packages in 17s

269 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
➜  vitess git:(frances/migrate-from-create-react-app) git status
On branch frances/migrate-from-create-react-app
nothing to commit, working tree clean
➜  vitess git:(frances/migrate-from-create-react-app) git pull origin main
From github.com:planetscale/vitess
 * branch                  main       -> FETCH_HEAD
Already up to date.

[dbussink]

@dbussink we wouldn't need to disable it if the workflow uses this branch's generate-proto-types.sh file 🤔 wasn't sure if that was the case or not.

It would be, afaik all jobs run against the current branch including this one so it would generate the same code.

[notfelineit]

@dbussink not sure why the CI is failing then 🤔 even after pulling latest from main + running make vtadmin_web_proto_types the proto files don't change - I think they're already up-to-date. CI is saying they are not.

Running locally:

➜  vitess git:(frances/single-component) ✗ ./tools/check_make_vtadmin_web_proto.sh

up to date, audited 1375 packages in 2s

269 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
VTAdmin Web Protos are up to date

[dbussink]

@dbussink not sure why the CI is failing then 🤔 even after pulling latest from main + running make vtadmin_web_proto_types the proto files don't change - I think they're already up-to-date. CI is saying they are not.

Hmm, the reason it's not showing a diff is that the files are marked as generated / binary, so if you modify .gitattributes and remove the following entries:

web/vtadmin/src/proto/** linguist-generated=true
web/vtadmin/src/proto/*.js -diff
web/vtadmin/src/proto/*.ts -diff

Then it should show what the difference is that CI sees so we can find out what's up here.

[notfelineit]

@dbussink thank you! Updating now and checking the diff. Hoping it shows!

[dbussink]

@notfelineit Should we revert this now that the issue was found?

[notfelineit]

Yes! 😄

[dbussink]

@notfelineit I'm seeing also a following message:

npm WARN deprecated [email protected]: react-flow-renderer has been renamed to reactflow, please use this package from now on https://reactflow.dev/docs/guides/migrate-to-v11/

Should we also update all dependencies here in general? Or do that in a separate PR then?

[notfelineit]

I'm thinking we can do that in a separate PR! Should be easy to upgrade now we're on Vite 😄