[Debt] Migrate to vite #10611
[Debt] Migrate to vite #10611
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10611 +/- ##
============================================
- Coverage 38.17% 38.09% -0.08%
- Complexity 1645 1646 +1
============================================
Files 1019 1021 +2
Lines 31339 31338 -1
Branches 6623 6646 +23
============================================
- Hits 11963 11938 -25
+ Misses 19351 19223 -128
- Partials 25 177 +152
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Sentry. |
|
Okay, I think things look better now, let us get some other eyes on this π |
There was a problem hiding this comment.
π₯³ this is so great!
β perhaps it is worth enabling the Chromatic workflow and running it on this PR as well, if the intent is for this PR to resolve that issue?
Important
my approval is limited to local testing (similar to @vd1992's implicit approval). i would wait to merge until @petertgiles is confident with deploying and testing on remote servers.
|
Build error in router.tsx π’ |
Not really sure why this occurred π . But it is now fixed with split experience form pages into own files |
This comment was marked as resolved.
This comment was marked as resolved.
It is subtle but important, dev command changed the Dev mode strips all this out to create a more optimized build for production. As for watch, the request number is so high becuase of how the HMR works. It dynamically loads chunks of code into the browser on demand and does this often (on every file save) to update the page. |
This reverts commit 3be63bf.
|
Thanks for answering I noticed that there is a Playing around with the various scripts, think we had a bug in one of them
Playwright still works well, as does codegen and tsc |
It is set in the chromatic workflow. The folder name is the standard name so it just gets picked up there.
It is a little confusing but, it is pointing to the correct location. The script is chekcing web but it is run in that package since it uses the |
π€ Resolves #9840
π Introduction
Migrates from webpack to vite.
π΅οΈ Details
Watch vs Dev/Build
Thanks to @substrae we do not have to sacrifice the watch command. However it now works a little different. When running
pnpm run dev/buildeverything works as you should expect. But in order to watch, we use vite's dev server for HMR. This means, when running watch, you should be usinghttp://localhost:3000(note the port number).Now, when developing, you no longer need to refresh the browser! Vite will reload the browser and reload the components. It does this on demand so you may notice some flashing and temporary error pages but I think this is a good trade off.
Barrel exports
It may be time to revisit removing all barrel exports again since vite discourages it in their performance section. We do not do this too much but I think we still have some.
π Breaking Changes
HTML Template
This has been moved to the root (expected by vite).
Environment variables
This has changed significantly. We no longer use the
config.ejsfile for runtime variables since vite supportsejsin thehtmltemplate.To add an environment variable, you still add it to the
.envbut instead of updated theconfig.ejsfor runtime vars, you need to add it to:vite.config.tsvite-env.d.tsindex.html(for runtime only)To access an environment variable, just reference it like a global constant. We no longer need to use the
process.envanymore.β Bonus
Storybook
This seems to also resolve #10557 so it would benefit us for you to test that as well π
Codegen watch
I was also able to figure out why codegen watch was broken and fixed that here as well since it was only 2 character change π
π§ͺ Testing
.env.exampleitems (DEV_APP_URL,DEV_OAUTH_POST_LOGIN_REDIRECT,API_HOST)make clean-modulespnpm ipnpm run build