-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: replace CRA with Vite [LIBS-598] #847
Conversation
BREAKING CHANGE: Supported Node versions are 18.x or 20+
reporter.info('Compiling service worker...') | ||
await compileServiceWorker({ config, paths, mode }) |
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.
Previously, CRA compiled the service worker in production builds. We already have our own compile function that we use for dev builds, so we use that here now
const relative = normalizeExtension(path.relative(inputDir, source)) | ||
const relative = path.relative(inputDir, source) |
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.
This was what broke JSX files in platform apps/libs: as an example, consider index.jsx
has import App from 'App.jsx'
. At compile time, App.jsx
gets converted to App.js
, but the import text in index
is still from App.jsx
(it doesn't get modified despite the filename change), and then an error is thrown that says "Can't find App.jsx
"
@@ -82,12 +81,11 @@ exports.verifyEntrypoints = ({ | |||
|
|||
const getEntrypointWrapper = async ({ entrypoint, paths }) => { | |||
const relativeEntrypoint = entrypoint.replace(/^(\.\/)?src\//, '') | |||
const outRelativeEntrypoint = normalizeExtension(relativeEntrypoint) |
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.
This was also mishandling .jsx
entrypoints
.replace(/'.\/D2App\/app'/g, `'./D2App/${outRelativeEntrypoint}'`) | ||
.replace(/'.\/D2App\/app\.jsx'/g, `'./D2App/${relativeEntrypoint}'`) |
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 placeholder filename in the shell is now called app.jsx
// Skip index.html, (plugin.html,) and `static` directory; | ||
// CRA's workbox-webpack-plugin handles it smartly | ||
globIgnores: [ | ||
'static/**/*', | ||
paths.launchPath, | ||
paths.pluginLaunchPath, | ||
// skip moment locales -- they result in many network requests and | ||
// slow down service worker installation | ||
'**/moment-locales/*', | ||
'**/*.map', |
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.
A few changes in this file now that it's taking over from CRA
// (see https://regex101.com/r/z4Hy9k/1/ for RegEx details) | ||
dontCacheBustURLsMatching: /\.[a-z0-9]{8}\.|\d+\.\d+\.\d+/, | ||
// (see https://regex101.com/r/z4Hy9k/3/ for RegEx details) | ||
dontCacheBustURLsMatching: /[.-][A-Za-z0-9-_]{8}\.|\d+\.\d+\.\d+/, |
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.
Updated for the chunks emitted from Vite's Rollup
// Precache all of the assets generated by your build process. | ||
// Their URLs are injected into the manifest variable below. | ||
// This variable must be present somewhere in your service worker file, | ||
// even if you decide not to use precaching. See https://cra.link/PWA. | ||
// Includes all built assets and index.html | ||
// Injection point for the precache manifest from workbox-build, | ||
// a manifest of app assets to fetch and cache upon SW installation |
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.
A few things are cleaned up in this file now that our own compilation script is handling service workers instead of CRA's
// Note that the CRA precache manifest files start with './' | ||
// TODO: Make this extensible with a d2.config.js option | ||
export const CRA_MANIFEST_EXCLUDE_PATTERNS = [ | ||
/^\.\/static\/js\/moment-locales\//, | ||
] |
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.
This was used to modify the manifest output by CRA; it's no longer needed
<!-- The entrypoint for Vite --> | ||
<script type="module" src="./src/index.jsx"></script> |
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.
Vite wants index.html
to be here at the root directory level. This script tag is the entrypoint for the rest of the app
@@ -17,7 +17,7 @@ | |||
] | |||
}, | |||
"devDependencies": { | |||
"@dhis2/cli-style": "^10.4.3", | |||
"@dhis2/cli-style": "^10.5.2", |
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.
@@ -29,7 +29,7 @@ | |||
"@babel/preset-react": "^7.0.0", | |||
"@babel/preset-typescript": "^7.6.0", | |||
"@dhis2/app-shell": "11.2.0", | |||
"@dhis2/cli-helpers-engine": "^3.2.0", | |||
"@dhis2/cli-helpers-engine": "^3.2.2", |
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.
<!-- | ||
manifest.json provides metadata used when your web app is installed on a | ||
user's mobile device or desktop. See https://developers.google.com/web/fundamentals/web-app-manifest/ | ||
--> | ||
<link | ||
rel="manifest" | ||
crossorigin="use-credentials" | ||
href="%PUBLIC_URL%/manifest.json" | ||
href="/manifest.json" |
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.
Vite resolves these URLs relative the the base
path specified in the Vite config
the only followup I have in mind, right now is to make it easier for consumers to convert from |
Looks like there are some simple one liners out there: https://gist.github.com/parties/90cdf35f9a3d05bea6df76dc83a69641 |
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.
Great stuff @KaiVandivier, allez vite!
# [12.0.0-alpha.1](v11.2.2...v12.0.0-alpha.1) (2024-06-06) ### Features * replace CRA with Vite [LIBS-598] ([#847](#847)) ([3dd0e59](3dd0e59)) ### BREAKING CHANGES * Supported Node versions are 18.x or 20+ * ci: upgrade Node version * fix: always add PWA_ENABLED to app env for better static analysis * chore: pause precache manifest injection * fix: building SW without CRA * chore: comment update * fix: group moment locales in their own dir * refactor: clean up precache injection * fix: locale utils and handling moment in jest * fix: compile correctly after merging changes * chore: comment in compile.js * chore: some clean-up * chore: comments * fix: use port 3000 for the dev server * fix: improve moment locale chunk naming * chore: remove CRA * fix: use mjs build of Vite * fix: bump cli-style for CRA fix * feat: use interactive dev server output from Vite * fix: make dev server port configurable * chore: remove old index.html * fix: env tweaks
🎉 This PR is included in version 12.0.0-alpha.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
# [12.0.0](v11.7.5...v12.0.0) (2024-12-13) ### Bug Fixes * **deps:** upgrade app-runtime and ui ([#895](#895)) ([8ed0ec3](8ed0ec3)) * **deps:** upgrade react to 18 in example apps ([#900](#900)) ([7fd16d7](7fd16d7)) * **deps:** use npm v6 before publishing ([01ad502](01ad502)) * **init:** update bootstrap script branch ([#896](#896)) ([33c261a](33c261a)) * **plugin:** clean up resize observer and handle sonarqube warnings ([#898](#898)) ([f113dd5](f113dd5)) * alerts from plugins [LIBS-695] ([#881](#881)) ([21be0d2](21be0d2)) * allow serving files from cwd node_modules ([0233949](0233949)) * base url env & refactor [LIBS-635] ([#872](#872)) ([7f19259](7f19259)) * bump ui library ([#882](#882)) ([1ae9569](1ae9569)) * clear only required build dirs ([#894](#894)) ([179305f](179305f)) * env refactor for Vite wrap-up [LIBS-690] ([#889](#889)) ([84da4e6](84da4e6)) * injectPrecacheManifest warning logging ([a0d266e](a0d266e)) * normalize to .js extensions when compiling libraries ([#893](#893)) ([58b33a8](58b33a8)) * **service-worker:** handle undefined config vars in injectPrecacheManifest ([a90a4e0](a90a4e0)) * correct app shell paths ([#883](#883)) ([a1af71c](a1af71c)) * handle jsx in js support [LIBS-633] ([#871](#871)) ([595a35d](595a35d)) * increase precache max file size to 3 MB ([b20ed22](b20ed22)) * remove custom eslint from TS template ([71cef4b](71cef4b)) * update deps ([1e7ce93](1e7ce93)) * update workbox deps to avoid deprecation warnings ([9a81c4a](9a81c4a)) * use strings for 'boolean' env vars ([eaf5e66](eaf5e66)) ### Features * create initial TS template ([#868](#868)) ([2795f79](2795f79)) * enable HMR for .js files ([5f4683c](5f4683c)) * handle plugins with Vite [LIBS-610] ([#863](#863)) ([ca5be0d](ca5be0d)) * jsx migration script ([#869](#869)) ([7764f49](7764f49)) * migrate snap files too ([#878](#878)) ([521f483](521f483)) * replace CRA with Vite [LIBS-598] ([#847](#847)) ([3dd0e59](3dd0e59)) * upgrade react to v18 ([#876](#876)) ([77ecf10](77ecf10)) * **init:** set direction: 'auto' and import locales for new apps [LIBS-645] ([#867](#867)) ([4eda4a9](4eda4a9)) ### BREAKING CHANGES * require react version 18 * fix: pin react version to 18 * test: update test and test libraries for react 18 * Supported Node versions are 18.x or 20+ * ci: upgrade Node version * fix: always add PWA_ENABLED to app env for better static analysis * chore: pause precache manifest injection * fix: building SW without CRA * chore: comment update * fix: group moment locales in their own dir * refactor: clean up precache injection * fix: locale utils and handling moment in jest * fix: compile correctly after merging changes * chore: comment in compile.js * chore: some clean-up * chore: comments * fix: use port 3000 for the dev server * fix: improve moment locale chunk naming * chore: remove CRA * fix: use mjs build of Vite * fix: bump cli-style for CRA fix * feat: use interactive dev server output from Vite * fix: make dev server port configurable * chore: remove old index.html * fix: env tweaks
🎉 This PR is included in version 12.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Part 1 of the changes -- LIBS-598
Includes:
react-scripts
is removed.[jt]sx
extensions in app files.jsx
extensions@dhis2/cli-helpers-engine
to let Vite's dev server use the console, which enables helpful interactive outputFollowing changes will look like:
import.meta.env.DHIS2_
env vars in our code (fromprocess.env
)env
var management, for exampleIf I remember right, these are the relevant breaking changes so far:
3. Hot-reloading of React components in files without a
.[jt]sx
extension is not really supported, unfortunately. This is the largest change. I added some workarounds invite.config.mjs
to allow compiling of JSX syntax in files without.[jt]sx
extensions, so apps can at least start once they install this new version, but consumers will need to change their file extensions to get the full HMR behavior. This design by Vite is intended to save computations by not needing to run a JSX parser on files without JSX, so it's limited to.[jt]sx
files. There are more notes inside the config file. I'm investigating easy ways like a codemod to run through a repo and update filenames of files containing JSX4. Required Node versions will be
^18 || >=20
5. Removing
react-scripts
revealed a bug in@dhis2/cli-style
-- it was missing a dependency thatreact-scripts
filled. Consumers will need to upgrade@dhis2/cli-style
to^10.5.2
6. The start script does not open the browser by default. This is the default Vite behavior, but I'm curious about input on this -- I like it, because opening the browser annoys me personally (since it doesn't work well on Firefox and Brave, the browsers I use), but maybe other people depend on it
7. Custom
index.html
files with%PUBLIC_URL%
and%REACT_APP_...%
replacements will break --%DHIS2_...%
variables will now work, and public URL is no longer needed. See the relative URL format inindex.html
in the shellHere are some other general notes:
.tsx
and it worked smoothly (albeit with many type errors).mjs
extension to avoid a warning about thecjs
package of Vite being deprecated