Switch testing from mocha/chai/karma/c8 to vitest. - #56
Draft
davidlehn wants to merge 24 commits into
Draft
Conversation
- Test on Node.js >=22. - Update `engines.node` to `>=22`. - Update README requirements section.
- Improve formatting. - Add more common README sections.
- `kyOriginalPromise` no longer exported. - `ky` is again exported. - Change from using `ky` promises to regular instances.
- After `ky@2` update the error body is already available in `error.data` and trying to get JSON again will fail.
- Remove `push`. - Add `query`, `options`, and `trace` to align with `ky@2`.
ky@2 merges header options via a plain object spread when both sides are still plain objects, which does not dedupe names that differ only by case (e.g. `Accept` vs `accept`), causing values to be appended instead of overridden. Use a `Headers` instance instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Fix import. - Use suggested ChromeHeadless options for CI.
Webpack's browser field remaps `tests/utils.js` to `tests/utils-browser.js`, but the browser stub never defined `makeAgent`. The namespace import in the shared spec file only references it inside an `isNode` guard, but webpack still statically validates the export, breaking the karma build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adding a non-simple header (e.g. `Authorization`) triggers a browser CORS preflight `OPTIONS` request, which this route never answered, causing the actual request to be blocked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`ky@2` wraps the browser's `TypeError: Failed to fetch` in its own `NetworkError`, with the original error moved to `cause`. Check both locations so the friendly CORS message still gets applied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Launch the browser with `--ignore-certificate-errors` so it accepts the self-signed cert, letting the local HTTPS test server test run in both node and browsers. This covers TLS in the browser without depending on an external site. Restrict the github.com test to node. The site sends no CORS headers, so a browser blocks the request before it is sent. Keeping it node-only also halves how often it runs, reducing rate limit exposure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`engines` requires node >=22, so the node 18.2+ guard on agent conversion is always true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Karma is unmaintained. Vitest replaces the runner, the browser test harness, and the coverage tool with a single dependency and config. - Add `vitest.config.js` with `node` and `browser` projects. The browser project runs Chromium through `@vitest/browser-playwright`, replacing karma and karma-webpack. - Keep the existing `should`-style assertions unchanged. `tests/setup.js` installs the global `should` from vitest's re-exported chai, so `chai` is no longer a dependency. - Start the HTTP/HTTPS test servers in `tests/globalSetup.js` and pass their ephemeral hosts to tests with `inject()`. This replaces starting them in the karma config and injecting the hosts via webpack's `DefinePlugin`, and lets `tests/utils-browser.js` be removed. - Split the suite by environment into `10-client-api.spec.js` (shared), `20-node.spec.js`, and `30-browser.spec.js` instead of branching on `isNode` at runtime. This keeps node-only modules out of the browser project and drops the `detect-node` dependency. Test bodies are unchanged; both projects still run 17 tests. - Report coverage with `@vitest/coverage-v8` across both projects, so browser-only code paths are now covered. Reported totals shift slightly because vitest and c8 count executable lines differently. - Replace the `test-karma` CI job with `test-browser`, and install and cache Playwright's Chromium in the browser and coverage jobs. Add an `exports` field with a `browser` condition for `agentCompatibility` and import it via a self-reference. Vite does not apply the top-level `browser` field to package-internal relative imports, so this is also a fix for browser bundlers, which would otherwise pull `undici` into their builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
davidlehn
force-pushed
the
remove-cjs-support
branch
from
August 6, 2026 23:22
80807f3 to
bb0518d
Compare
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
1 participant
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.
Karma is unmaintained. Vitest replaces the runner, the browser test harness, and the coverage tool with a single dependency and config.
vitest.config.jswithnodeandbrowserprojects. The browser project runs Chromium through@vitest/browser-playwright, replacing karma and karma-webpack.should-style assertions unchanged.tests/setup.jsinstalls the globalshouldfrom vitest's re-exported chai, sochaiis no longer a dependency.tests/globalSetup.jsand pass their ephemeral hosts to tests withinject(). This replaces starting them in the karma config and injecting the hosts via webpack'sDefinePlugin, and letstests/utils-browser.jsbe removed.10-client-api.spec.js(shared),20-node.spec.js, and30-browser.spec.jsinstead of branching onisNodeat runtime. This keeps node-only modules out of the browser project and drops thedetect-nodedependency. Test bodies are unchanged; both projects still run 17 tests.@vitest/coverage-v8across both projects, so browser-only code paths are now covered. Reported totals shift slightly because vitest and c8 count executable lines differently.test-karmaCI job withtest-browser, and install and cache Playwright's Chromium in the browser and coverage jobs.Add an
exportsfield with abrowsercondition foragentCompatibilityand import it via a self-reference. Vite does not apply the top-levelbrowserfield to package-internal relative imports, so this is also a fix for browser bundlers, which would otherwise pullundiciinto their builds.