Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] frontend: Replace CRA with Vite #1696

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

tazo90
Copy link

@tazo90 tazo90 commented Feb 5, 2024

What I did

This PR is related to the issue #1282.

Changes apply only to the /frontend directory and replace CRA with Vite 5 for development and production builds.
Thanks to Vite cold start is much faster and gives better DX.

Because of failing tests, this PR is still in progress.

Benchmarks:
CRA:
cold start: 1m 30s
build: 1m 50s

Vite:
cold start: 2.5s
build: 48s

Overview:

  • Remove CRA (react-scripts)
  • Integrate Vite 5 for development sever and production build
  • Upgrade Storybook to v8 with Vite
  • Optimize the production bundle by using smaller chunks
  • Visualize dependencies (analysis.html produced after build via rollup-plugin-visualizer)
  • Fix tests - fails 11 of 19

Detailed migration steps:

  • Remove react-scripts
  • Remove %PUBLIC_URL%
  • Rename REACT_APP_ environment variables to VITE_
  • Install Vite and add vite.config.ts
  • Install Jest and add jest.config.ts
  • Update scripts section in package.json
  • Replace process.env with import.meta.env and move envs to global constants
  • Upgrade Storybook to v8 and setup Vite
  • Get rid of imports via require

Known issues:

  • @storybook/addon-storyshots is deprecated. See migration guide.
  • Legacy dev server warnings
  • Failing tests - Vite and Jest do not work well together, it would be good to migrate to vitest.

Proposition of further steps:

  • Replace Jest with Vitest
  • Remove webpack (check relationships and potential impact)

@tazo90 tazo90 marked this pull request as draft February 5, 2024 21:38
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
Signed-off-by: Mariusz Winnik <[email protected]>
@tazo90 tazo90 force-pushed the feat/1282-replace-cra-with-vite branch from 974361a to 8f4b425 Compare February 5, 2024 21:47
@tazo90 tazo90 force-pushed the feat/1282-replace-cra-with-vite branch from 8f4b425 to 5b97f9e Compare February 5, 2024 21:56
@tazo90 tazo90 changed the title [WIP] Replace CRA with Vite [WIP] frontend: Replace CRA with Vite Feb 5, 2024
@joaquimrocha
Copy link
Collaborator

Hi @tazo90 . Thanks for the PR. As indicated in #1282 , we still need to assess what should be the right project to replace CRA as there are also some implications in terms of how to do special builds (with branding, etc.). So we may take a while to review this PR.

@tazo90
Copy link
Author

tazo90 commented Feb 6, 2024

Hi @joaquimrocha.

You're welcome! I completely understand the need for a thorough assessment of the potential replacements for CRA.
It might be a good idea to wait for the final decision before improving Jest-based tests or introducing vitest.
I'm available for any further questions or discussions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants