Replace Karma with Vitest and finish the Rollup removal - #42764
Merged
Conversation
The llms.txt work fails the spellcheck on a word that is now part of the project vocabulary. Unblocks #42728
The `.browserslistrc` comment claimed about 90.8% coverage, but `npx browserslist --coverage` measured 77.53%. Two things caused the gap. The `>=` lines are joined with OR, so they never acted as floors, and caniuse gives the newest Safari a null release date, which hides roughly 8.2 points of real usage from every range query. Adding `unreleased versions` recovers that Safari share at zero output cost, because it is newer than every floor. Lowering the floors to the true native `light-dark()` boundary reaches about 88% for 1.68 kB of CSS, all of it `-webkit-backdrop-filter`. We stop there on purpose: the next rung up drops below native `light-dark()`, where Lightning CSS falls back to the custom-property polyfill that broke `data-bs-theme`. Dropping and_uc, and_qq and kaios costs 0.718% and removes the only reason Autoprefixer emitted `-webkit-mask-*` and `-moz-column-gap`, so `removeRedundantPrefixes` now only trims the prefixed `transition` declarations that come from the prefixed thumb selectors. With the list reduced to an explainable set, `build/browser-targets.mjs` can resolve it at build time instead of restating the floors by hand.
The workflow has been `disabled_manually` with zero runs since about October 2024, so this has not tested anything for well over a year. The browser matrix pinned Chrome 60, Firefox 60, iOS 12 and Android 8, all far below the v6 floors, so re-enabling it would only fail on modern syntax. Dropping the launcher also removes 34 packages from the lockfile, including the legacy tunnel wrapper and its unzip stack. The README credit goes too, since we no longer use the service. `ip` and `karma-jasmine-html-reporter` stay: `npm run js-debug` still needs both.
Both configs bundle the already-built dist, so no TypeScript is involved and the switch is mechanical. `@rollup/plugin-replace` becomes `transform.define`, and node resolution is built in, so the plugin stack disappears. They now share `build/browser-targets.mjs` with the dist build instead of carrying their own Babel targets. Also drops the `js/tests/integration/rollup*.js` glob from the ESLint config. It never matched anything, because the files are `.mjs`.
Vitest 4 ships browser mode as stable, so the specs now run in a real Chromium through Playwright. The suite drops from about 25 seconds to under 6, and the runner needs no Rollup or Babel stack of its own. The specs are written against Jasmine, so `js/tests/vitest-setup.js` maps that API onto Vitest rather than rewriting roughly 22,000 lines. The one behaviour that cannot be codemodded is `spyOn`: Jasmine stubs the method, Vitest calls through, and 92 call sites here rely on the stub. A blind conversion would leave tests that pass while asserting nothing, so the shim stubs by default and restores the real method only for `.and.callThrough()`. hammer-simulator assigns to `event.target`, which throws in the strict mode a module gets, so it runs through a Function constructor to keep the sloppy-mode behaviour Karma gave it. Coverage now instruments all of `js/src` instead of only the files a spec imports. That is a larger and more honest denominator, and it immediately showed `combobox.ts` at 0%, so this adds the missing spec. The branch threshold moves from 89 to 88 to match the new denominator; statements, functions and lines all still clear 90. Vitest 4 also removed the `done` callback, so the four remaining callback-style specs now return a promise.
Karma was the last thing using them. With the specs on Vitest and the dist builds and integration bundles on Rolldown, nothing reads `.babelrc.mjs` or the Rollup TypeScript resolver any more, so both files and all nine packages go. Rollup and Babel still appear in the lockfile as transitive dependencies, because Vite bundles Rollup and the Istanbul coverage provider instruments through Babel. Neither is ours to configure now. Also records the deliberate `rolldown` pin. It is exact-pinned while still release-candidate software, so Dependabot should not move it without review.
Nine `as unknown as` casts and seven `as any` casts hid two different problems behind the same syntax. Seven of the `as unknown as` casts passed a boolean to `setAttribute`, which takes a string. A `setAriaAttribute` helper does that conversion in one place, so callers no longer reach for a cast. The two that remain are unrelated and stay: one writes to `element.style`, the other is a not-set config sentinel. The datepicker casts were bridging Bootstrap's wide `string` and `number` config types to the narrow literal unions Vanilla Calendar Pro actually accepts. Narrowing `DatepickerConfig` to those unions removes all seven and turns an out-of-range value into a compile error instead of a silent no-op inside the calendar. One assertion survives, where `getMonth()` returns a number TypeScript cannot prove is 0-11, but it now names the type it narrows to. Also deletes the stale `selector-engine.ts` TODO. It claims `next()` is unused, but `prev()` and `next()` have three call sites each across `menu.ts`, `scrollspy.ts` and `combobox.ts`, and both are tested.
The constructor carried a TODO saying it should throw in v6, and this is v6. Until now it returned early and handed back an instance that looked fine but did nothing, so a markup mistake produced tabs that silently refused to switch. This is a breaking change for anyone constructing a Tab outside a `.list-group`, `.nav` or `[role="tablist"]` container. The old test asserted the silent return by name, so it now asserts the throw instead.
`api.ts` and `consumer.ts` only exercised 11 of the 20 plugins, so the public surface of the other nine was never asserted. Both files now cover all of them. The datepicker assertions are the point of this. Four negative cases check that the narrowed Vanilla Calendar Pro unions reject out-of-range values such as `firstWeekday: 7` or `displayMonthsCount: 13`. Extending `consumer.ts` matters more than `api.ts` here, because it reads the emitted declarations and so proves the unions survive the `.d.ts` emit rather than widening on the way out. Also adds a Dialog case for a prevented `show` event, which was an uncovered branch.
`js/tests/README.md` still described Karma and Jasmine, and its paths said `tests/unit/` where the real path is `js/tests/unit/`. It now covers Vitest browser mode and calls out the two things that will catch a contributor out: `spyOn` stubs by default unlike `vi.spyOn`, and Vitest 4 dropped the `done` callback. The coverage section explains why the branch floor sits at 88 and why an untested plugin now reports 0% instead of vanishing from the report. `contribute.mdx` listed Sass, PostCSS and terser but named no JavaScript bundler at all, so it gains a section on Rolldown and on how `build/browser-targets.mjs` keeps both pipelines on one browser list. `migration.mdx` gets the same switch from the consumer's angle, since it only matters to people who build from source. `AGENTS.md` is updated to match.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
`js/tests/types/` holds compile-time assertions. `tsc` checks them and nothing ever runs or bundles them, so the quality suite's unused-variable query fires on essentially every line: the typed declaration is the assertion, and using the variable afterwards would add nothing. The files were already full of these before this branch. They only became visible because CodeQL annotates changed lines on a pull request, so extending the coverage to all 20 plugins lit up 40 notices for a pattern that was already there.
This branch lowered the browserslist floors, but the browsers and devices page still reproduced the old ones. It listed Chrome and Edge 130, Firefox 132, and Safari 18, where the real minimums are now Chrome and Edge 123, Firefox 120, and Safari 17.5. The rationale was stale in two more ways. It claimed the floors covered roughly 90% of users, a figure that came from browserslist.dev and its pinned caniuse data; `npx browserslist --coverage` reports 87%. It also said we could not lower the floors, which this branch did. Each floor now sits on the first version of that engine with native `light-dark()`, so the real constraint is the polyfill that breaks `data-bs-theme`. Also note that browserslist ORs its lines, because the page implied `last 2 major versions` was the whole policy, and record that we now exclude UC Browser, QQ Browser, and KaiOS.
A floor applies to a whole major line. We cannot raise it in a minor release, so whatever 6.0 ships caps what every later 6.x feature may use. Earlier in this branch I set the floors on the first versions with native `light-dark()`, which bought 2.18 points of coverage but left v6.x with the oldest baseline we could tolerate rather than the one we want. The low Firefox floor was also wrong on its own terms. `:has()` needs Firefox 121, so Firefox 120 sat inside our support window and could not render our forms, which use `:has()` 28 times in validation alone. The new floors sit on the newest feature v6 already depends on: `:has()` at Firefox 121, `content-visibility` at Firefox 125 and Safari 18, `<details name>` at Firefox 130, `text-wrap: balance` at Chrome 130, and unprefixed `backdrop-filter` at Safari 18. `light-dark()` stays the hard lower bound. Coverage drops from 87.19% to 85.01%. The JavaScript is byte-identical, because every version in both sets handles the syntax we emit. The CSS loses the `-webkit-backdrop-filter` copies on the Dialog and Drawer backdrops, so it fits under the old bundlewatch limit again and this branch no longer needs to raise it.
The file already explained which features set each floor. It did not say what the floors give us in return, so the cost was documented and the benefit was not. Anyone reading it could see why we cannot go lower without learning what they may safely use. Add two lists. The first names the features that need no fallback, prefix, or polyfill in any 6.x release. The second names the ones we still cannot use, with the version that blocks each. Firefox is the blocker in almost every case. Relative color syntax is the closest at Firefox 133, and anchor positioning is the one worth waiting for, because it would let Menu, Tooltip, Popover, and Combobox drop Floating UI. Every version here comes from the caniuse-lite copy in this repo, checked against the current floors. Comments only, so the resolved target list and the build output do not change.
The migration guide never mentioned browser support. That was the largest gap in it, because v5 supported Chrome and Firefox 60 and Safari 12, while v6 now needs Chrome and Edge 130, Firefox 132, and Safari 18. A reader with an older audience cannot adopt v6 at all, and nothing in the guide said so. Add a warning callout to the upgrade steps rather than a changelog bullet, because this is a prerequisite to check before starting, not a change to apply while working. It links to the browsers and devices page for the reason behind each version. The approach page embeds `.browserslistrc` in full, so the rationale comments added in the last commit now render there ahead of the queries. Point that page at the summary table, so a reader who only wants the versions does not have to scroll the comments.
The rationale had grown to roughly 50 comment lines, and the approach page embeds this file in full, so all of it rendered there ahead of the queries it was meant to explain. Prose belongs on a page that can format it. Move the parts the docs did not already cover onto the browsers and devices page: why `last 2 major versions` carries our mobile coverage, why `unreleased versions` is worth eight points, how to measure coverage and why not to trust browserslist.dev, which features the floors do and do not give us, and which prefixes the excluded engines would add. Leave behind a pointer to that page and the two rules a person editing the file must not get wrong. Both pages now read the query lines out of the file through `fileMatch` instead of hand-copying them. That is what drifted in the first place: the page still claimed Chrome 130 and Safari 18 after the floors briefly moved to 123 and 17.5. The regex is anchored on the first and last query, so a restructure fails the docs build rather than shipping stale versions. The resolved target list, the derived oxc targets, and the build output are all unchanged.
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
2 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.
Replaces Karma with Vitest, finishes the Rollup and Babel removal, and clears the type-safety, browser-target, and docs debt that went with them.
js/tests/vitest-setup.jsmaps the Jasmine API onto Vitest instead of rewriting ~22,000 lines of specs; notespyOnstubs by default there, unlikevi.spyOn.js/src, not just the files a spec imports. The larger denominator moves the branch threshold 89 → 88; statements, functions, and lines still clear 90. It also exposedcombobox.tsat 0%, so this adds the missing spec (42 tests).@rollup/plugin-*packages,rollup-plugin-istanbul, all four@babel/*packages,.babelrc.mjs, andbuild/rollup-plugin-ts-resolve.cjs, and port the two integration bundles to Rolldown. Both stay in the lockfile as transitive deps, since Vite bundles Rollup and the Istanbul provider instruments through Babel.js-test-cloud, dependabot and ncurc entries, README credit. Zero runs since ~October 2024, and its matrix pinned Chrome 60 / Firefox 60 / iOS 12.npx browserslist --coveragereported 77.53%. Floors apply to a whole major line, so these pin to the newest feature v6 already needs::has()(Firefox 121),content-visibility(Firefox 125),<details name>(Firefox 130),text-wrap: balance(Chrome 130), unprefixedbackdrop-filter(Safari 18). Anything lower is unsound — Firefox 120 predates:has(), which form validation alone uses 28 times.light-dark()stays the hard lower bound (Fix dark mode on built site by bumping browserslist #42471).unreleased versions, which recovers ~8 points of real Safari usage at zero output cost, and derive the build targets inbuild/browser-targets.mjsfrom.browserslistrcinstead of restating the floors by hand.as unknown ascasts into asetAriaAttributehelper, and narrowDatepickerConfigto the Vanilla Calendar Pro literal unions, removing all sevenas anycasts. Out-of-range values are now compile errors, not silent no-ops.Tabthrows aTypeErrorwhen its element has no.list-group,.nav, or[role="tablist"]ancestor, instead of returning an inert instance that hid the markup error.js/tests/types/api.tsandconsumer.tsfrom 11 of 20 plugins to all 20, including datepicker negative cases against the emitted declarations..browserslistrcthroughfileMatch, so the published versions cannot drift again. Updatejs/tests/README.md,AGENTS.md, andcontribute.mdxfor Vitest and Rolldown.js/tests/types/from CodeQL, since those compile-time assertions trip the unused-variable query by design. Drop the staleselector-engine.tsTODO and addllmsto the cspell dictionary.Two pre-existing bugs surfaced while writing the combobox spec, both out of scope here:
Comboboxbinds itself to the toggle, then constructs aMenuon that same element, so the secondData.setis refused andMenu.getInstance()on the toggle returns null. Source of the repeatedBootstrap doesn't allow more than one instance per elementerrors during the run.EventHandler.onstrips the namespace from native event names whiletriggerdispatches the full namespaced name, so listeners for events likechange.bs.comboboxnever fire. The spec works around it with nativeaddEventListener, followingdatepicker.spec.js.Unblocks #42728.