-
Notifications
You must be signed in to change notification settings - Fork 20
Migrate to using Vite #487
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
Conversation
✅ Deploy Preview for tsml-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
generate lcov set node version Add helper test
d719801 to
e9773d3
Compare
Add pettier migrate prettier globals rm babel config
|
I think I have addressed all of the previous issues and concerns with previous effort to move to
|
use iife simplify config
|
I also successfully reproduced and verified the fix using the Recovery Dharma production site (https://recoverydharma.org/meetings/) by saving the complete HTML locally and testing both builds: the broken flat bundle format produced |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @pjaudiomv ! works well for me locally. i noticed the bundle size went from 590kb with mix to 586kb with vite.
this is especially great because mix is officially EOL so some vulnerabilities were starting to accumulate.
also thank you for doing that testing on the RD website 🙌
| <div | ||
| id="tsml-ui" | ||
| data-src="https://sheets.code4recovery.org/storage/12Ga8uwMG4WJ8pZ_SEU7vNETp_aQZ-2yNVsYDFqIwHyE.json" | ||
| data-timezone="America/New_York" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not clear on what this file is doing - we do want to preserve this page: https://tsml-ui.code4recovery.org
it still works for me locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separate index.html at the root is strictly for dev (npm run dev with HMR) and public/index.html stays there and is for the actual site page. This was the cleanest way to have HMR working and the separate public/index.html site. The files in public/tests/ cant be used with hmr (though viewing works fine for npm run dev) as need to have the one valid entrypoint. They can be tested with npm run previewafter build though ornpm run serve`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TL;DR its needed for dev entrypoint, public/index.html is still preserved for https://tsml-ui.code4recovery.org/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't have HMR on multiple entry points in a single dev server (the public/tests/*.html files) so we have to pick one test to develop against with HMR (the root index.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
closes #486 |
fix doc numbering
This PR aims to modernize the build and development files.
Changes
Laravel MixandWebpack. AddsVitegiving us faster builds and HMRJest. AddsVitestwith native ESM support andViteintegration.eslintrc.jsto flat config (eslint.config.js).prettierrctoprettier.config.jsNotes
npm run formatshould prob be ran in subsequent PR, I chose not to run it as didn't want to create larger diff than necessary. We could then add checks to github ci for prettier to pass.Development Workflow
Before (Laravel Mix):
After (Vite):
Benefits
Testing this (my fork)
Option 1: Add new remote to your existing tsml-ui repo
Option 2: Just clone fork