Skip to content

build: migrate bundler and test runner from webpack/Jest to Vite/Vitest - #236

Merged
meseer merged 9 commits into
mainfrom
build/migrate-vite-vitest
Oct 30, 2026
Merged

meseer merged 9 commits into
mainfrom
build/migrate-vite-vitest

Conversation

@meseer

@meseer meseer commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Migrate bundler from webpack + Babel to Vite 8 (vite.config.mjs), preserving single-file IIFE output (dist/monarch-uploader.user.js), Terser ascii_only: true minification, and UserScript metadata header injection (src/userscript-metadata.cjs via rollupOptions.output.banner in dev builds for accurate inline source maps and post-minify in production builds)
  • Raise minimum browser build target from legacy .babelrc (Chrome 61 / Firefox 60) to explicit build.target: ['chrome100', 'firefox100', 'safari16.4'] for modern evergreen browsers running Violentmonkey/Tampermonkey (keeping native ES2022 ?. / ?? / async-await syntax without downleveling)
  • Migrate test runner and coverage from Jest + Babel + c8 to Vitest 5 + @vitest/coverage-v8 (pool: 'vmThreads', reducing full suite execution time from ~24s to ~5s)
  • Convert test-file CommonJS require() / jest.requireMock() calls to ESM imports and jest.* APIs to vi.*
  • Extract debugLog into src/core/logger.ts and replace lazy require('./integrationRegistry') cycles in src/ with standard ESM imports
  • Remove legacy dependencies (webpack, webpack-cli, terser-webpack-plugin, babel-loader, @babel/core, @babel/preset-env, @babel/preset-typescript, jest, jest-environment-jsdom, c8) and config files (webpack.config.cjs, jest.config.cjs, .babelrc)
  • Add npm run typecheck step to .github/workflows/ci.yml and update tsconfig.json, eslint.config.mjs, knip.json, .github/dependabot.yml, and project documentation
  • Bump version to 8.0.3

Test Plan

  • npm run build:full passes with zero errors or warnings
  • All 174 test suites (4,984 tests) pass under Vitest (pool: 'vmThreads')
  • test/build/bundleEncoding.test.js verifies pure 7-bit ASCII output, UserScript metadata block, Terser ascii_only config, end-to-end dev inline source map VLQ line resolution after the banner, and isolated new JSDOM(..., { url }) runtime boot smoke tests through DOMContentLoaded and initApp across all supported institutions
  • Validated dist/monarch-uploader.user.js locally

Mykhailo Delegan added 5 commits October 29, 2026 16:40
- Replace top-level require()/jest.requireMock() with import declarations
- Hoist in-function require()/jest.requireMock() to namespace imports
- Use dynamic import() after resetModules()/doMock()
- Convert rogersBankPage and wealthsimpleAuthCookie helpers to ESM exports
- Prepares the suite for Vitest, where vi.mock only intercepts ESM imports
- Add vite, vitest, @vitest/coverage-v8, and jsdom devDependencies
- Configure Vitest in vite.config.mjs (jsdom, globals, setupFiles, v8 coverage)
- Extract debugLog into src/core/logger.ts and replace CJS require() cycles in src/ with ESM imports
- Replace jest.* APIs with vi.* across test/ and add default export shims to vi.mock factories
- Fix unawaited setTimeout callbacks in test/ui/categorySelector.test.js
- Update package.json test, test:coverage, and test:watch scripts to use Vitest
- Configure Vite IIFE library build in vite.config.mjs (entry src/index.ts, output dist/monarch-uploader.user.js)
- Add userscript-banner plugin to prepend src/userscript-metadata.cjs post-minify
- Configure Terser minifier with ascii_only: true and userscript comment preservation
- Update build and watch scripts in package.json to use vite build
- Update test/build/bundleEncoding.test.js to validate the Vite build and Terser config
- Uninstall webpack, webpack-cli, terser-webpack-plugin, babel-loader, @babel/*, jest, jest-environment-jsdom, and c8
- Delete webpack.config.cjs, jest.config.cjs, and .babelrc
- Update tsconfig.json types to include vitest/globals
- Update eslint.config.mjs globals from jest to vitest
- Update knip.json and .github/dependabot.yml for Vite and Vitest
- Replace @jest-environment docblocks with @vitest-environment in test/ui/
…o 8.0.3

- Update AGENTS.md, .claude commands/skills, and integration runbook
- Update entry point and integration barrel docstrings for Vite
- Bump version from 8.0.2 to 8.0.3
@meseer

meseer commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

Review

Nothing blocks merging. There's one real bug, which only affects dev builds, plus a few smaller fixes and some nits.

What I checked:

  • Tests: all 174 test files (4,977 tests) pass under Vitest. No test was removed or skipped. The 37 changed tests only changed their signature (sync → async, or done callback → async).
  • Typecheck and knip: tsc passes with no errors, and knip reports no unused or unlisted dependencies.
  • Bundle size: the production bundle is 768 KB, against 786 KB from webpack on main.
  • Smoke test: I loaded the new bundle and main's bundle in jsdom with GM stubs on Monarch, Wealthsimple, MBNA, Neo, Rogers and an unrelated site. Neither threw on any of them. This matters because the PR replaces the delayed require() calls with normal imports, which changes when modules load.

P1: fix before merge

  1. Dev-build source maps point about 30 lines too low (vite.config.mjs, userscriptBannerPlugin). The plugin adds the ~30-line banner to file.code in generateBundle but doesn't update the source map. In a dev build, line 39455, 30 lines above the real call, resolves to src/index.ts:120, close to the debugLog('Script loaded successfully') call at line 119. Webpack's BannerPlugin kept the maps correct. This affects build:dev, watch and watch:dev, not production. Two possible fixes:
    • use output.banner when minify is off;
    • add the banner in a renderChunk hook with MagicString and return the map.

P2: should address

  1. The minimum browser version went up without being mentioned. The deleted .babelrc compiled down to Chrome 61 / Firefox 60. With no build.target set, Vite 8 uses its default (roughly Chrome 111 / Firefox 114 / Safari 16.4). The new bundle keeps 662 ?. and 200 ?? unconverted. That's probably fine for userscript users, but it should be a decision: either set build.target explicitly or note it in the PR. It also affects whether 8.0.3 should be a patch or a minor bump.
  2. Comments about an import cycle are now wrong. src/core/integrationCapabilities.ts (near getCapabilities) and src/core/utils.ts (in getDefaultLookbackDays) describe a cycle that "ESM live bindings resolve" because "integrationRegistry imports utils too". This PR moved integrationRegistry.ts to import only ./logger, so that cycle no longer exists. Drop the comments, or replace them with one line saying logger.ts is what keeps the registry free of cycles.
  3. CI doesn't run typecheck. ci.yml runs lint, test:coverage and build only. This PR changes tsconfig.json types to vitest/globals, and nothing in CI checks that. This predates the PR, but it's cheap to add a step now.

P3: nits and cleanup

  1. rollupOptions.output.extend: true and lib.name do nothing: src/index.ts has no exports, so the output is a bare !function(){…}() with no global. The whole rollupOptions block can go.
  2. vite.config.mjs sets the timezone twice: process.env.TZ at the top level and test.env.TZ. Keep one. As written, it also sets the timezone during vite build.
  3. In eslint.config.mjs, the '*.config.mts' ignore matches no file, and 'vite.config.mjs' is already covered by '*.config.mjs'.
  4. utils.ts uses import * as integrationRegistry, while accountService, configStore and integrationCapabilities use named imports. Pick one style.
  5. src/index.ts now casts reg as unknown as …, which exposes a real mismatch between the registry's IntegrationApi and generic UI's ApiClient types. Fine for this PR; worth a follow-up issue.
  6. Test files still have about 70 __esModule: true entries in vi.mock factories, which Vitest ignores. Also, the vi.doMock('../../src/api/monarch', …) in categorySelector.test.js's beforeEach has no effect, since the module was already imported statically. That was true under Jest as well.
  7. Optional speed-up: Vitest reports that jsdom setup takes 74% of the test run (98s summed across files, with 26s wall clock). Its own suggestion is pool: 'vmThreads'.
  8. Optional: add a jsdom boot check for the built bundle under test/build/, next to bundleEncoding.test.js. Nothing currently runs the built bundle, so a future change to module loading order could break the real bundle while every test still passes.

🤖 Generated with Claude Code

Mykhailo Delegan added 2 commits October 29, 2026 17:23
- Inject userscript banner via rollupOptions.output.banner in dev/watch builds so inline source maps include the banner line offset
- Set explicit build.target (chrome100, firefox100, safari16.4) in vite.config.mjs
- Remove duplicate top-level process.env.TZ assignment and unused rollupOptions.output.extend in vite.config.mjs
- Enable pool: 'vmThreads' in Vitest config (cutting test run time from ~24s to ~5s) and remove global.window reassignment in monarchLoginLink.test.js
- Add typecheck step to .github/workflows/ci.yml
- Clean up outdated cycle comments in integrationCapabilities.ts and utils.ts, and use named imports in utils.ts
- Remove redundant ignore entries in eslint.config.mjs
- Remove unused __esModule: true properties across test mocks and dead vi.doMock in categorySelector.test.js
- Add jsdom runtime boot smoke tests and dev banner source-map test to test/build/bundleEncoding.test.js
@meseer

meseer commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

Review, round 2 (0d81c0a)

The new revision fixes almost everything from the first review. The one real problem left is that the new boot smoke test doesn't test what it says it does. I'd fix that before merging; everything else is minor.

What I checked:

  • npm run build:full passes, and so does lint, typecheck and test:coverage (the steps CI runs). There are 4,984 tests now, 7 more than before.
  • The test run dropped from about 26s to 5–9s with pool: 'vmThreads'.
  • The production bundle is still 768 KB and boots cleanly on all six hosts in a separate jsdom check I ran outside the test suite.

Fixed

# Item How I checked
1 Dev source maps off by ~30 lines Fixed. In a dev build, generated lines now resolve exactly to src/index.ts:119 and src/core/logger.ts:12.
2 Minimum browser version Set explicitly: chrome100 / firefox100 / safari16.4.
3 Stale cycle comments Fixed, and utils.ts now uses named imports.
4 CI typecheck Added to ci.yml.
5–7, 10 Leftover config, duplicate timezone setting, ESLint ignores, __esModule entries, dead vi.doMock All cleaned up; no __esModule entries remain.
6 Timezone now set only when mode === 'test' Tests see America/Vancouver whether the shell has no TZ, UTC or Asia/Tokyo, with and without coverage. The full suite also passes under TZ=Asia/Tokyo.

P1: the boot smoke test checks much less than it claims

test/build/bundleEncoding.test.js, runtime boot smoke test (jsdom):

  1. Every case runs on localhost.
    • global.mockLocation in test/setup.js expects an object, but the test passes a URL string. The string is spread into keys like '0', '1', …, and href falls back to http://localhost/.
    • The helper doesn't work even when given { href }: window.location.hostname still reads localhost under the vmThreads, threads and forks pools alike. jsdom doesn't allow location to be overridden.
    • This test is the only caller of mockLocation, so nothing exposed the problem.
    • Result: the six site cases are one identical case, and no site-specific code path runs.
  2. initApp never runs. jsdom's readyState is 'complete', so src/index.ts:114 schedules initApp with setTimeout(…, 1000). vi.clearAllTimers() in finally then discards it. The test only covers evaluating the bundle and setting up the early hooks.
  3. Each case runs the bundle again on the same shared window. Side effects pile up across the six cases, such as the Rogers XHR wrapper and extra event listeners.

Suggested fix: create a fresh new JSDOM('<!doctype html>…', { url, runScripts: 'outside-only' }) for each case, with GM stubs. jsdom is already a dev dependency. Then eval the bundle, dispatch DOMContentLoaded, advance past 1s, and assert that there were no error events and no console.error calls.

P3: nits

  1. The dev-banner test asserts config details (plugins equals [], rollupOptions.output.banner is a function) instead of the result. Building the dev bundle and checking one known source line through the map would test the actual fix and survive refactors.
  2. The comment on build.target says "without legacy polyfill bloat", but Vite never adds polyfills. It only converts newer syntax for older browsers. Something like "keeps native ES2022 syntax" would be accurate.
  3. vmThreads is a big speed-up, but Vitest documents caveats: memory growth, and instanceof checks on built-in types can fail across contexts. The suite passes today. Keep it in mind if odd instanceof failures appear later.
  4. Optional: the PR description still doesn't mention the minimum browser change (Chrome 61 / Firefox 60 → 100 / 100). That's worth a line, and it bears on whether this should be a patch or a minor bump.

🤖 Generated with Claude Code

…ify dev source map VLQ mapping

- Create a fresh JSDOM instance per site URL in test/build/bundleEncoding.test.js, dispatch DOMContentLoaded, advance timers through initApp (1000ms), and assert zero errors and site-specific initialization logs
- Build the development bundle in bundleEncoding.test.js and decode its inline VLQ source map to verify line alignment with src/index.ts after the userscript banner
- Remove dead broken mockLocation/restoreLocation helpers from test/setup.js and expose Web Streams globals for JSDOM under vmThreads
- Clarify build.target comment in vite.config.mjs
@meseer

meseer commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

Review, round 3 (e748cdf)

This revision fixes the round 2 P1. I'd merge after one small follow-up: the smoke test checks its results before any async work runs, so it misses errors logged asynchronously.

What I checked:

  • npm run build:full passes: 4,984 tests, no warnings.
  • Nothing else in the repo uses the removed mockLocation / restoreLocation helpers.

Fixed

  • The smoke test now really runs each site.
    • Each case gets its own new JSDOM(..., { url }) and asserts the hostname.
    • It simulates a page that is still loading, dispatches DOMContentLoaded, then runs the queued timers up to 1000ms, so initApp now runs.
    • It asserts the per-site "Running on …" log message.
  • The source-map test now checks the result: it builds the dev bundle and decodes the inline map down to the src/index.ts line.
  • The broken mockLocation / restoreLocation helpers were removed from test/setup.js.
  • The build.target comment was corrected.

How strong the new smoke test is

I planted failures at the top of initApp and ran bundleEncoding.test.js against each one:

Planted failure Result
throw new Error(...) (sync) ✅ All site cases fail
Promise.resolve().then(() => { throw … }) ⚠️ Vitest reports "Unhandled Rejection" and the run fails, but not because of the test's own check (see item 2)
Promise.resolve().then(() => console.error(...)) ❌ Not caught: all 19 tests pass

P2

  1. Async errors during startup aren't caught. The test is synchronous: it asserts errors / consoleErrors right after draining the timers, before any queued promise callbacks run. Then dom.window.close() runs. Anything initApp starts asynchronously is never checked. The fix is small: make the test async and add await new Promise((r) => setImmediate(r)) (or a few such ticks) before the expects.
  2. The unhandledrejection listener never fires. jsdom doesn't dispatch that event; I checked with a standalone JSDOM and Promise.reject, and the listener stayed silent. Rejections are only caught by Vitest's own global handler, which fails the run without pointing at the test. Either remove the listener or collect rejections some other way. One option is to wrap the window's Promise rejection path; another is to rely on the await from item 1 plus console.error.

P3: nits

  1. The timer loop has no cap on iterations. while (timeouts.length > 0) re-runs any callback with a delay of 1000ms or less, including ones queued during the loop. If startup ever gets a poll that reschedules itself (e.g. setTimeout(poll, 500)), the loop spins forever. The test's timeout can't interrupt a synchronous loop, so the worker hangs instead of failing. Add a cap such as for (let i = 0; i < 100 && timeouts.length; i++).
  2. Timers run in the order they were queued, not by delay. A 1000ms callback queued before a 0ms one runs first. That's harmless today, but sorting by delay would match real browsers.
  3. The Rogers case uses rbaccess.rogersbank.com, but the userscript only matches selfserve.rogersbank.com/*. The test passes because index.ts checks includes('rogersbank.com'), but the real script never runs on that host. Use selfserve.rogersbank.com.
  4. expect(menuCommands).toHaveLength(5) will break whenever a menu command is added. Assert specific command names, or at least > 0.
  5. The test hand-writes a ~35-line source-map decoder. It's correct for this purpose, so this is fine to leave. source-map-js is already installed as a dependency of Vite, so adding it as a dev dependency would replace those lines with three. Your call.

🤖 Generated with Claude Code

- Make runtime boot smoke test async and flush setImmediate ticks after eval, DOMContentLoaded, each timer step, and initApp so async console.error calls and promise rejections are caught
- Capture unhandled promise rejections via process.on('unhandledRejection') instead of a no-op window listener
- Order queued timers by dueAt on a virtual clock with a 100-step cap and add Questrade and Canada Life cases
- Use https://selfserve.rogersbank.com/home for Rogers Bank and assert menuCommands contains 'Clear All Cached Data'
@meseer

meseer commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

Review, round 4 (f283dd2)

This revision fixes everything raised in round 3, and I found nothing new worth holding the merge for. Ready to merge.

What I checked:

  • npm run build:full passes with 4,986 tests and no warnings. The smoke tests take about 2.7s.
  • All CI checks on the PR pass, including the required test (24.x).
  • The new Questrade and Canada Life cases use hosts covered by the userscript's @match patterns, and Rogers now uses selfserve.rogersbank.com.

Round 3 items, all fixed

# Item Fix
1 Async errors not caught The test is now async and lets pending promise work finish after each step and at the end.
2 unhandledrejection listener never fires Replaced with process.on('unhandledRejection'), which is removed in finally.
3 Timer loop could hang Capped at 100 steps, with an expect(steps).toBeLessThan(MAX_TIMER_STEPS) check.
4 Timers run in queue order A simulated clock now runs them in order of due time and stops past 1000ms.
5 Rogers host doesn't match @match Now selfserve.rogersbank.com/home.
6 Hard-coded toHaveLength(5) Now toContain('Clear All Cached Data').
7 Hand-written source-map decoder Kept. That was optional, so fine.

Planted failures

I put each of these at the top of initApp and re-ran the test:

Planted failure Result
Sync throw ✅ All 8 boot cases fail
Async console.error ✅ All 8 fail (round 3: not caught)
Async promise rejection ✅ All 8 fail, and the failure now points at the test
Poll that reschedules itself every 0ms ✅ All 8 fail through the step cap; no hang
Poll that reschedules itself every 500ms ✅ Passes, which is correct: the clock stops at 1000ms

Leftover observations (not worth another round)

  • setInterval is stubbed out and never fires, so code driven by intervals isn't exercised. That's an accepted limit of a boot smoke test.
  • The process.on('unhandledRejection') listener would also catch an unrelated rejection from elsewhere in the same worker while a case runs. Cases in a file run one after another, so the risk is low.

🤖 Generated with Claude Code

@meseer
meseer merged commit f16c60e into main Oct 30, 2026
7 checks passed
@meseer
meseer deleted the build/migrate-vite-vitest branch October 30, 2026 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant