feat(Build): modernize build and test tooling with Vite/Vitest & oxlint/oxfmt#3435
Open
daker wants to merge 38 commits into
Open
feat(Build): modernize build and test tooling with Vite/Vitest & oxlint/oxfmt#3435daker wants to merge 38 commits into
daker wants to merge 38 commits into
Conversation
daker
force-pushed
the
vite-vitest-migration
branch
29 times, most recently
from
65d7ab0 to
825c998
Compare
The old webpack (UMD via style-loader) and rollup (ESM via rollup-plugin-postcss with default inject:true) both injected stylesheets as <style> tags on import. The Vite migration silently dropped this: UMD emitted vtk.js.css alongside vtk.js, and ESM emitted orphan .module.css files that nothing imported, so UI/widget consumers lost their styling without any warning. - inlineUmdCssPlugin: add enforce:'post' so generateBundle sees the CSS asset emitted by Vite's css-post plugin. Without it, the bundle filter returned empty and the plugin no-op'd. - injectEsmCssPlugin: new plugin that inlines each *.module.css into its corresponding *.module.css.js wrapper as a <style> tag injected on first evaluation, then deletes the orphan stylesheet from the bundle. Verified by diffing against the published @kitware/vtk.js@35.15.1 artifacts, which inject CSS via styleInject; the new wrappers behave equivalently.
Application demos are emitted as standalone single-file HTML (per each app's index.md: "the only requirement is the single HTML file without any web server"). The old Rollup path ran terser; the Vite migration disabled minification, bloating each demo from ~1.5 MB to ~2.5 MB. Re-enable esbuild minification, which is the fast path Vite already bundles, no extra dependency. Readable source for learning still lives in Examples/Applications/* on GitHub; the inlined script in the deployable HTML was never intended as a view-source artifact.
The old webpack ESLint plugin honored NOLINT to skip linting during the release build (since lint runs separately on the line). The Vite migration has no equivalent consumer, so the env var is dead config.
The previous cssRuntimePlugin hand-rolled ~130 lines of CSS Modules logic — regex class-name parsing, sha256 hashing, manual composes resolution — that re-implemented (incompletely) what Vite handles via postcss-modules. Cross-file composes, nested rules, complex selectors, and pseudo-classes were all silent footguns waiting for a contributor to write a more complex CSS module. Let Vite do the CSS parsing and class-name scoping. A new ~25-line inlineExtractedCssPlugin walks chunk.viteMetadata.importedCss at generateBundle time, inlines each chunk's extracted CSS as a <style> injection IIFE, and deletes the orphan asset. Same end behavior as the old plugin (CSS bundled into the standalone HTML, class names work), but with Vite's battle-tested CSS Modules implementation underneath. Also set assetsInlineLimit:Infinity on the ES module build to match the application build; the old plugin always inlined url() refs as data URIs, and extracted assets would otherwise resolve to /_assets/ paths that don't honor the VitePress base prefix. Net: -196 / +41 lines.
Forrest Li added matched ESM and UMD .d.ts validation configs in 2022 (c925926); both have been in CI ever since. The Vite migration kept the ESM half but dropped the UMD half, leaving the UMD .d.ts rewriting pipeline (copyUmdAssetsPlugin in vite.config.js) with no in-repo check. The UMD .d.ts files use absolute "vtk.js/Sources/..." imports rewritten from relative paths at build time. If that rewriting breaks, the only signal today is downstream TypeScript users hitting unresolvable imports. Restore the config and wire it into both build-test and publish workflows so the next .d.ts regression fails at PR time.
Two related broken URL behaviors in the generated examples:
generate-examples.mjs hard-coded the iframe src and full-screen <a>
href to '${name}/index.html' without forwarding window.location.search
or window.location.hash, so navigating to examples/SkyboxViewer.html
?fileURL=... loaded the example wrapper but the iframe inside got the
URL without any parameters. Add a <script setup> block with an
onMounted hook that re-binds the iframe and link to the page URL plus
the current search/hash; Vue's :src/:href bindings then update both.
develop_webxr.md linked to legacy paths like GeometryViewer/Geometry-
Viewer.html and WebXRVolume/WebXRVolume.html that the new build never
emits. After the new build the working paths are GeometryViewer.html
(the VitePress wrapper, which forwards its query string to the iframe
per the change above) and WebXRVolume.html. Rewrite all ten broken
references and the four nested ones inside fileURL=[...] values.
The Testing sidebar items hard-coded link: '/vtk-js/coverage/...', but VitePress automatically prepends base: '/vtk-js/' to absolute links. The built sidebar ended up with /vtk-js/vtk-js/coverage/... and docs:build logged "No matching file" warnings. Drop the manual prefix so VitePress emits the intended single-prefixed path. Coverage report files are still generated separately by CI and served at /vtk-js/coverage/ on the deployed site; this fix only corrects the sidebar link the docs site emits.
docs:generate writes ~200 files into Documentation/api/, ~179 into Documentation/examples/, and a Documentation/examples/gallery.js manifest. None were gitignored, so a casual `git add Documentation/` after running the docs pipeline locally would commit hundreds of build artifacts. Both directories keep one hand-authored index.md that stays tracked via negated patterns. sidebar.ts is left tracked-as-placeholder for now (config.ts imports it; gitignoring needs a fresh-checkout strategy that's out of scope).
The Vite migration dropped the slimmed-down vtk-lite.js UMD bundle. It was never documented but was published to npm and CDN for years, and at least one downstream (sphinxcontrib-cadquery) vendors it. Copy vtk.js to vtk-lite.js so existing <script src=.../vtk-lite.js> and CDN consumers keep working. Byte-identical so the shared sourceMappingURL still resolves vtk.js.map. Adds ~2.65MB to the UMD npm tarball — accepted as a one-version deprecation alias. BREAKING_CHANGES.md notes the alias and the one behavior change worth flagging: anything indexing into the ColorMaps array by position now sees the full preset set instead of the lite subset.
vite.config.js had grown to ~450 lines holding six inline Rollup
plugins (copyEsmAssetsPlugin, copyUmdAssetsPlugin,
generateDtsReferencesPlugin, cleanupAssetsPlugin, injectEsmCssPlugin,
inlineUmdCssPlugin) plus five filesystem helpers and the
SOURCE_IGNORE_LIST/ignoreSourceFile pair. Move all of that to a new
Utilities/build/vtk-plugins.mjs alongside the existing generic
plugins.mjs, so vite.config.js becomes config wiring only (~150
lines). Same closeBundle/generateBundle behavior, including the
vtk-lite.js alias and the .module.css inlining.
Drive-by cleanups while moving:
- Extract flattenIndexEntry so both the ESM entryFileNames callback
and generateDtsReferencesPlugin use a single Foo/index -> Foo
helper instead of duplicate regexes.
- Tighten externals from new RegExp('^' + name) to ^name(/|$) so a
hypothetical @types/webxray-foo wouldn't be matched by the
@types/webxr pattern. Today nothing collides, but the old form
was a quiet footgun.
- Replace the hand-rolled copyDir with fs.cpSync({recursive:true}),
stable in the Node 22 we already pin in CI.
- Share the style-injection IIFE template between the ESM and UMD
CSS plugins.
Also flip ESM sourcemap to true. UMD already had it; ESM was off
without a stated reason and the old rollup ESM build emitted them.
Per-file .map files are siblings of each preserveModules chunk.
The migration left compareImages calling expect() with the condition as the value and the diagnostic message as the second positional arg, then chaining .toBeTruthy(): expect(minDelta < mismatchTolerance, '...').toBeTruthy(); This works because Vitest 4's expect(value, message) carries the message into the assertion error, but it reads like the old tape t.ok(cond, msg) shape and obscures both the matcher and the actual value on failure. Switch to expect(value, msg).toBeLessThan(...) so the failure report shows the actual delta percentage instead of just "true is not truthy". Mirror the dimensions check with .toBe(true).
example-runner-cli.js was CJS (require) that loaded Vite via
dynamic import, then spawned createServer({ configFile: ... })
pointing at vite.example.config.mjs (ESM). The two halves
communicated by mutating process.env with EXAMPLE_ENTRY,
EXAMPLE_NAME, EXAMPLE_HOST, etc., which the config file then read
back at module-load time. Mixing module formats in a modernization
PR is jarring, and the env-var protocol forces a singleton vite
process per CLI invocation.
Rewrite the CLI as .mjs and switch vite.example.config.mjs from a
defineConfig'd default export to an exported
createExampleConfig({ repoRoot, entry, name, host, port,
openBrowser, useHttps }) function. The CLI imports that function
and passes the result directly to createServer() with
configFile:false, so there's no env-var handoff and no file path
to keep in sync. Also expose --host and --port flags now that
they're cheap to thread through.
Run via the new path in package.json scripts:
example, example:https, example:webgpu
Local runs popped the Vitest browser-mode test runner UI every invocation. CI was already headless (vitest detects CI=1 automatically); set browser.headless explicitly so local matches. browser.headless suppresses both the UI tab and the test execution browser; setting it on the per-instance launch options only affects the latter.
Collaborator
Author
|
@PaulHax FF test are failing due to no webgl context, i still believe that trace: "retain-on-failure" is needed, maybe it will make Firefox tests passes when Playwright is "watching" either by recording videos or traces. |
Collaborator
Author
|
@PaulHax try : trace: 'retain-on-failure', |
Headless Firefox under Playwright disables WebGL when the GPU blocklist trips, which is always the case in GitHub Actions runners. The CI Firefox instance was launched with no firefoxUserPrefs, so every rendering test logged "no webgl context" and failed. Apply the same prefs in CI as locally (webgl.force-enabled plus disable-fail-if-major-performance-caveat) so xvfb-run + llvmpipe is accepted as a software renderer. Local mode already had these prefs; this just dedupes them and adds the CI branch.
- Remove xvfb-run from build-test.yml: every browser launches headless (browser.headless: true + chromium --headless flag from Playwright), so no X server is consumed. - Remove --headless=new from the chromium args; Playwright already passes the right headless flag for the version it ships. - Gate junit reporter on CI so local runs don't write Utilities/TestResults/junit-report.xml every invocation. - Add cache: 'npm' to the setup-node-project composite action so both workflows pick up the npm cache without duplication. - Drop "Running tests in CI mode" log; Vitest re-evaluates the config per browser instance so it printed twice and added nothing.
The script was `cross-env CI=1 vitest run`. GitHub Actions auto-sets CI=true, so the cross-env wrapper was a no-op in CI — its only caller. The misleading "reporter" name is a fossil from when this script toggled the junit reporter, which is now unconditional. Workflow now calls `npm test` directly.
…chromium Extract the Firefox instance object (browser + prefs) once at module scope and reference it from both CI and local branches. Drops the old quirk where firefoxUserPrefs were attached to local Chromium launch (Playwright ignored them; just noise).
Splits the single --with-deps install step into three: - Read playwright version from node_modules (post-npm-ci) for the cache key, so the cache invalidates exactly when the dep bumps. - actions/cache@v4 over ~/.cache/ms-playwright keyed on OS + version. - Browser binary download runs only on cache miss (~300MB skipped on hit). System libs (libgbm, ffmpeg, etc.) run every time since apt-installs aren't cacheable per job. Saves ~30-90s per CI run.
PR Kitware#15 in this fork proved Firefox WebGL passes with per-instance launch.headless: true + xvfb-run. The earlier simplifications here broke Firefox because Vitest's global browser.headless follows a different launch path (Firefox drops WebGL on Linux CI runners). - Restore xvfb-run --auto-servernum on the test step. - Restore per-instance launch.headless: true for both chromium and firefox (CI and local). - Bump actions/checkout v4→v6, setup-node v4→v6, cache v4→v5, upload-artifact v4→v7 to silence the Node 20 deprecation warning without falling back to FORCE_JAVASCRIPT_ACTIONS_TO_NODE24.
Three followups surfaced by /simplify review: - publish.yml was still calling the deleted `test:reporter` script and using v4 actions. Switch it to `npm test` and bump checkout v4→v6 / upload-artifact v4→v7 to match build-test.yml. - Cache key for Playwright browsers uses matrix.os (ubuntu-24.04) so the cache invalidates if the runner image is bumped (runner.os is just "Linux" and would reuse a stale cache after an OS upgrade). - vitest.config.js: tighten the header comment and the Firefox-prefs inline notes — same WHY, half the lines.
Collaborator
Author
|
@PaulHax FF tests failed again 😭 i am not sure if FF WebGL in headless mode is working https://bugzilla.mozilla.org/show_bug.cgi?id=1375585 |
Set `screenshotDirectory: 'Utilities/TestResults/screenshots'` on the Vitest browser provider so failure screenshots land where the existing CI test-results artifact upload already picks them up. Add `.vitest-attachments/` to .gitignore — that's the transient dir Vitest writes other browser-test attachments into, which doesn't need to be tracked.
The Vite-migration build wrote `"type": "module"` into the published
package.json and stopped copying `Utilities/config/`. Two regressions
vs `@kitware/vtk.js@35.15.2`:
1. Shipped CommonJS files (webpack-config helpers, bin CLIs) failed to
load with `ReferenceError: require is not defined`.
2. `Utilities/config/*` helpers vanished — webpack/vue-CLI consumers
relying on
`require('@kitware/vtk.js/Utilities/config/rules-vtk')` etc. broke.
Restore both:
- Copy `Utilities/config/` into `dist/esm` (alongside the existing
`Utilities/XMLConverter` and `Utilities/DataGenerator` copies).
- Write a nested `{"type": "commonjs"}` package.json into each of the
three CJS subdirs so their `.js` files load via `require()` despite
the root `"type": "module"`. Node respects nearest-ancestor scope.
Add a CI smoke step that packs `dist/esm`, installs the tarball in a
scratch dir, then exercises three previously-regressed surfaces:
extension-less `require()` of two config helpers and `--help` on the
`vtkDataConverter` bin.
Collaborator
|
I think we got the FireFox CI issue by getting the config plumming right so it gets a headed browser in 39f2b94 |
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This PR completes a tooling migration from the legacy webpack/rollup + Karma/Tape setup to a unified Vite + Vitest stack. It updates local development, build output, test execution, and CI integration to run on the new toolchain.
Results
Changes
PR and Code Checklist
npm run reformatto have correctly formatted codeTesting