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

feat: replace CRA with Vite [LIBS-598] #847

Merged
merged 32 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
93799a6
feat: serve app with Vite
KaiVandivier Apr 16, 2024
554ac0b
fix: leave compilation to Vite to enable HMR and .jsx files
KaiVandivier Apr 22, 2024
d6b6b44
chore: remove log
KaiVandivier Apr 22, 2024
94c23eb
fix: convert app shell files to .jsx
KaiVandivier Apr 22, 2024
2ee8abf
feat: build apps with Vite
KaiVandivier Apr 22, 2024
8d1286b
chore: clean up config
KaiVandivier Apr 22, 2024
ba7b412
fix: still compile for libs for now
KaiVandivier Apr 23, 2024
3501da2
fix: dynamic import & bundle splitting of moment locales
KaiVandivier Apr 24, 2024
e30508d
chore: bump required Node versions
KaiVandivier Apr 24, 2024
c35653a
ci: upgrade Node version
KaiVandivier Apr 24, 2024
ceace1b
fix: always add PWA_ENABLED to app env for better static analysis
KaiVandivier Apr 24, 2024
b196bd9
chore: pause precache manifest injection
KaiVandivier Apr 24, 2024
178164f
fix: building SW without CRA
KaiVandivier Apr 26, 2024
e4dd729
chore: comment update
KaiVandivier Apr 26, 2024
067ad3f
fix: group moment locales in their own dir
KaiVandivier Apr 26, 2024
1233956
refactor: clean up precache injection
KaiVandivier Apr 26, 2024
710503e
fix: locale utils and handling moment in jest
KaiVandivier Apr 27, 2024
8d05509
Merge branch 'alpha' into libs-598-replace-cra-with-vite
KaiVandivier Apr 27, 2024
429f61f
fix: compile correctly after merging changes
KaiVandivier Apr 29, 2024
a256fdf
chore: comment in compile.js
KaiVandivier Apr 29, 2024
60e9b64
chore: some clean-up
KaiVandivier Apr 30, 2024
c20b9db
chore: comments
KaiVandivier Apr 30, 2024
ae77246
fix: use port 3000 for the dev server
KaiVandivier Apr 30, 2024
78f8832
fix: improve moment locale chunk naming
KaiVandivier May 2, 2024
73a44f9
chore: remove CRA
KaiVandivier May 6, 2024
5380a6e
fix: use mjs build of Vite
KaiVandivier May 6, 2024
6f2b0bd
fix: bump cli-style for CRA fix
KaiVandivier May 6, 2024
a0ccf72
feat: use interactive dev server output from Vite
KaiVandivier May 8, 2024
6e34184
fix: make dev server port configurable
KaiVandivier May 8, 2024
293933b
Merge branch 'master' into libs-598-replace-cra-with-vite
KaiVandivier May 8, 2024
dd4a524
chore: remove old index.html
KaiVandivier May 8, 2024
ac84759
fix: env tweaks
KaiVandivier Jun 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/dhis2-deploy-netlify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 18.x

- uses: c-hive/gha-yarn-cache@v1
- run: yarn install --frozen-lockfile
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/dhis2-verify-lib.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 18.x

- uses: c-hive/gha-yarn-cache@v1
- run: yarn install --frozen-lockfile
Expand All @@ -43,7 +43,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 18.x

- uses: c-hive/gha-yarn-cache@v1
- run: yarn install --frozen-lockfile
Expand All @@ -59,7 +59,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 18.x

- uses: actions/download-artifact@v2
with:
Expand All @@ -81,7 +81,7 @@ jobs:
token: ${{env.GH_TOKEN}}
- uses: actions/setup-node@v1
with:
node-version: 14.x
node-version: 18.x

- uses: actions/download-artifact@v2
with:
Expand Down
7 changes: 4 additions & 3 deletions adapter/src/utils/localeUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,10 @@ export const setMomentLocale = async (locale) => {

for (const localeName of localeNameOptions) {
try {
await import(
/* webpackChunkName: "moment-locales/[request]" */ `moment/locale/${localeName}`
)
// Since Vite prefers importing the ESM form of moment, we need
// to import the ESM form of the locales here to use the same
// moment instance
await import(`moment/dist/locale/${localeName}`)
moment.locale(localeName)
break
} catch {
Expand Down
3 changes: 3 additions & 0 deletions cli/config/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ module.exports = {
transform: {
'^.+\\.[t|j]sx?$': require.resolve('./jest.transform.js'),
},
// Need to apply babel transformations to modules under moment/dist/,
// which use ES module format. See localeUtils.js in the adapter
transformIgnorePatterns: ['/node_modules/(?!moment/dist/)'],
moduleNameMapper: {
'\\.(css|less)$': require.resolve('./jest.identity.mock.js'),
'\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
Expand Down
4 changes: 2 additions & 2 deletions cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@dhis2/cli-app-scripts",
"version": "11.2.2",
"engines": {
"node": ">=14"
"node": "^18.0.0 || >=20.0.0"
},
"repository": {
"type": "git",
Expand All @@ -29,7 +29,7 @@
"@babel/preset-react": "^7.0.0",
"@babel/preset-typescript": "^7.6.0",
"@dhis2/app-shell": "11.2.2",
"@dhis2/cli-helpers-engine": "^3.2.0",
"@dhis2/cli-helpers-engine": "^3.2.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"@jest/core": "^27.0.6",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.4",
"@yarnpkg/lockfile": "^1.1.0",
Expand Down
5 changes: 4 additions & 1 deletion cli/src/commands/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const parseConfig = require('../lib/parseConfig')
const { isApp } = require('../lib/parseConfig')
const makePaths = require('../lib/paths')
const makePlugin = require('../lib/plugin')
const { injectPrecacheManifest } = require('../lib/pwa')
const { injectPrecacheManifest, compileServiceWorker } = require('../lib/pwa')
const makeShell = require('../lib/shell')
const { validatePackage } = require('../lib/validatePackage')
const { handler: pack } = require('./pack.js')
Expand Down Expand Up @@ -139,6 +139,9 @@ const handler = async ({
}

if (config.pwa.enabled) {
reporter.info('Compiling service worker...')
await compileServiceWorker({ config, paths, mode })
Comment on lines +142 to +143
Copy link
Contributor Author

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


reporter.info(
'Injecting supplementary precache manifest...'
)
Expand Down
9 changes: 4 additions & 5 deletions cli/src/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,9 @@ const handler = async ({

if (config.pwa.enabled) {
reporter.info('Compiling service worker...')
await compileServiceWorker({
config,
paths,
mode: 'development',
})
await compileServiceWorker({ config, paths, mode })
// don't need to inject precache manifest because no precaching
// is done in development environments
}

reporter.print('')
Expand All @@ -132,6 +130,7 @@ const handler = async ({
)
reporter.print('')

// todo: split up app and plugin starts
const shellStartPromise = shell.start({ port: newPort })

if (config.entryPoints.plugin) {
Expand Down
10 changes: 4 additions & 6 deletions cli/src/lib/compiler/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,11 @@ const {
createAppEntrypointWrapper,
createPluginEntrypointWrapper,
} = require('./entrypoints.js')
const {
extensionPattern,
normalizeExtension,
} = require('./extensionHelpers.js')
const { extensionPattern } = require('./extensionHelpers.js')

const watchFiles = ({ inputDir, outputDir, processFileCallback, watch }) => {
const compileFile = async (source) => {
const relative = normalizeExtension(path.relative(inputDir, source))
const relative = path.relative(inputDir, source)
Comment on lines -20 to +17
Copy link
Contributor Author

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"

const destination = path.join(outputDir, relative)
reporter.debug(
`File ${relative} changed or added... dest: `,
Expand Down Expand Up @@ -125,7 +122,8 @@ const compile = async ({
watchFiles({
inputDir: paths.src,
outputDir: outDir,
processFileCallback: compileFile,
// todo: handle lib compilations with Vite
processFileCallback: isAppType ? copyFile : compileFile,
watch,
}),
isAppType &&
Expand Down
4 changes: 1 addition & 3 deletions cli/src/lib/compiler/entrypoints.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const path = require('path')
const { reporter, chalk } = require('@dhis2/cli-helpers-engine')
const fs = require('fs-extra')
const { isApp } = require('../parseConfig')
const { normalizeExtension } = require('./extensionHelpers.js')

const verifyEntrypoint = ({ entrypoint, basePath, resolveModule }) => {
if (!entrypoint.match(/^(\.\/)?src\//)) {
Expand Down Expand Up @@ -82,12 +81,11 @@ exports.verifyEntrypoints = ({

const getEntrypointWrapper = async ({ entrypoint, paths }) => {
const relativeEntrypoint = entrypoint.replace(/^(\.\/)?src\//, '')
const outRelativeEntrypoint = normalizeExtension(relativeEntrypoint)
Copy link
Contributor Author

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

const shellAppSource = await fs.readFile(paths.shellSourceEntrypoint)

return shellAppSource
.toString()
.replace(/'.\/D2App\/app'/g, `'./D2App/${outRelativeEntrypoint}'`)
.replace(/'.\/D2App\/app\.jsx'/g, `'./D2App/${relativeEntrypoint}'`)
Comment on lines -90 to +88
Copy link
Contributor Author

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

}

exports.createAppEntrypointWrapper = async ({ entrypoint, paths }) => {
Expand Down
10 changes: 5 additions & 5 deletions cli/src/lib/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module.exports = (cwd = process.cwd()) => {
readmeDefault: path.join(__dirname, '../../config/init.README.md'),

shellSource,
shellSourceEntrypoint: path.join(shellSource, 'src/App.js'),
shellSourceEntrypoint: path.join(shellSource, 'src/App.jsx'),
shellSourcePublic: path.join(shellSource, 'public'),

base,
Expand All @@ -55,17 +55,17 @@ module.exports = (cwd = process.cwd()) => {
i18nLocales: path.join(base, './src/locales'),

d2: path.join(base, './.d2/'),
appOutputFilename: 'App.js',
appOutputFilename: 'App.jsx',
shell: path.join(base, './.d2/shell'),
shellSrc: path.join(base, './.d2/shell/src'),
shellAppEntrypoint: path.join(base, './.d2/shell/src/App.js'),
shellAppEntrypoint: path.join(base, './.d2/shell/src/App.jsx'),
shellAppDirname,
shellApp: path.join(base, `./.d2/shell/${shellAppDirname}`),
shellPluginBundleEntrypoint: path.join(
base,
'./.d2/shell/src/plugin.index.js'
'./.d2/shell/src/plugin.index.jsx'
),
shellPluginEntrypoint: path.join(base, './.d2/shell/src/Plugin.js'),
shellPluginEntrypoint: path.join(base, './.d2/shell/src/Plugin.jsx'),
shellSrcServiceWorker: path.join(
base,
'./.d2/shell/src/service-worker.js'
Expand Down
21 changes: 10 additions & 11 deletions cli/src/lib/pwa/compileServiceWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ const getPWAEnvVars = require('./getPWAEnvVars')
* dir for use with a dev server. In production mode, compiles a minified
* service worker and outputs it into the apps `build` dir.
*
* Currently used only for 'dev' SWs, since CRA handles production bundling.
* TODO: Use this for production bundling as well, which gives greater control
* over 'injectManifest' configuration (CRA omits files > 2MB) and bundling
* options.
* This could be migrated to a Vite config. Note that it still needs to be
* separate from the main app's Vite build because the SW needs a
* single-file IIFE output
*
* @param {Object} param0
* @param {Object} param0.config - d2 app config
Expand All @@ -23,22 +22,22 @@ const getPWAEnvVars = require('./getPWAEnvVars')
*/
function compileServiceWorker({ config, paths, mode }) {
// Choose appropriate destination for compiled SW based on 'mode'
const outputPath =
mode === 'production'
? paths.shellBuildServiceWorker
: paths.shellPublicServiceWorker
const isProduction = mode === 'production'
const outputPath = isProduction
? paths.shellBuildServiceWorker
: paths.shellPublicServiceWorker
const { dir: outputDir, base: outputFilename } = path.parse(outputPath)

// This is part of a bit of a hacky way to provide the same env vars to dev
// SWs as in production by adding them to `process.env` using the plugin
// below.
// TODO: This could be cleaner if the production SW is built in the same
// way instead of using the CRA webpack config, so both can more easily
// share environment variables.
// TODO: This could be refactored to be simpler now that we're not using
// CRA to build the service worker
const env = getEnv({ name: config.title, ...getPWAEnvVars(config) })

const webpackConfig = {
mode, // "production" or "development"
devtool: isProduction ? false : 'source-map',
entry: paths.shellSrcServiceWorker,
output: {
path: outputDir,
Expand Down
7 changes: 6 additions & 1 deletion cli/src/lib/pwa/getPWAEnvVars.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ function stringifyPatterns(patternsList) {
* @param {Object} config
*/
function getPWAEnvVars(config) {
if (!isApp(config.type) || !config.pwa.enabled) {
if (!isApp(config.type)) {
return null
}
if (!config.pwa.enabled) {
// Explicitly adding this value to the env helps pare down code in
// non-PWA apps when doing static bundle analysis
return { pwa_enabled: 'false' }
}
return {
pwa_enabled: JSON.stringify(config.pwa.enabled),
pwa_caching_omit_external_requests_from_app_shell: JSON.stringify(
Expand Down
17 changes: 8 additions & 9 deletions cli/src/lib/pwa/injectPrecacheManifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,24 @@ function logManifestOutput({ count, filePaths, size, warnings }) {
* `workbox-build`.
*/
module.exports = function injectPrecacheManifest(paths, config) {
// See https://developers.google.com/web/tools/workbox/reference-docs/latest/module-workbox-build#.injectManifest
// See https://developer.chrome.com/docs/workbox/modules/workbox-build#injectmanifest_mode
const injectManifestOptions = {
swSrc: paths.shellBuildServiceWorker,
swDest: paths.shellBuildServiceWorker,
globDirectory: paths.shellBuildOutput,
globPatterns: ['**/*'],
// 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',
Comment on lines -45 to +49
Copy link
Contributor Author

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

...config.pwa.caching.globsToOmitFromPrecache,
],
additionalManifestEntries: config.pwa.caching.additionalManifestEntries,
injectionPoint: 'self.__WB_BUILD_MANIFEST',
injectionPoint: 'self.__WB_MANIFEST',
// Skip revision hashing for files with hash or semver in name:
// (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+/,
Copy link
Contributor Author

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

}

return injectManifest(injectManifestOptions).then(logManifestOutput)
Expand Down
2 changes: 2 additions & 0 deletions cli/src/lib/shell/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ module.exports = ({ port, ...vars }) => {
...filterEnv(),
...makeShellEnv(vars),
}),
...filterEnv(),
...makeShellEnv(vars),
PORT: port,
PUBLIC_URL: process.env.PUBLIC_URL,
}
Expand Down
7 changes: 4 additions & 3 deletions cli/src/lib/shell/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module.exports = ({ config, paths }) => {
build: async () => {
await exec({
cmd: 'yarn',
args: ['run', 'build'],
args: ['build'],
cwd: paths.shell,
env: getEnv({ ...baseEnvVars, ...getPWAEnvVars(config) }),
pipe: false,
Expand All @@ -31,10 +31,11 @@ module.exports = ({ config, paths }) => {
start: async ({ port }) => {
await exec({
cmd: 'yarn',
args: ['run', 'start'],
args: ['start'],
cwd: paths.shell,
env: getEnv({ ...baseEnvVars, port, ...getPWAEnvVars(config) }),
pipe: false,
// this option allows the colorful and interactive output from Vite:
stdio: 'inherit',
})
},
// TODO: remove? Test command does not seem to call this method
Expand Down
2 changes: 1 addition & 1 deletion examples/simple-app/d2.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const config = {
// standalone: true, // Don't bake-in a DHIS2 base URL, allow the user to choose

entryPoints: {
app: './src/App.js',
app: './src/App.jsx',
},

dataStoreNamespace: 'testapp-namespace',
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useDataQuery } from '@dhis2/app-runtime'
import moment from 'moment'
import React from 'react'
import { Alerter } from './Alerter.js'
import { Alerter } from './Alerter.jsx'
import style from './App.style.js'
import i18n from './locales/index.js'

Expand All @@ -23,6 +23,10 @@ const Component = () => {
<h1>{i18n.t('Hello {{name}}', { name: data.me.name })}</h1>
<h3>
{i18n.t('Have a great {{dayOfTheWeek}}!', {
// NB: This won't localize on a dev build due to
// Vite's monorepo dep pre-bundling behavior.
// `moment` localization works outside the monorepo
// and in production here though
dayOfTheWeek:
moment.weekdays(true)[moment().weekday()],
})}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
]
},
"devDependencies": {
"@dhis2/cli-style": "^10.4.3",
"@dhis2/cli-style": "^10.5.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"@dhis2/cli-utils-docsite": "^3.0.0",
"concurrently": "^6.0.0",
"serve": "^12.0.0"
Expand Down
Loading
Loading