Skip to content

Remove CJS support. - #52

Merged
davidlehn merged 20 commits into
update-depsfrom
remove-cjs-support
Aug 12, 2026
Merged

Remove CJS support.#52
davidlehn merged 20 commits into
update-depsfrom
remove-cjs-support

Conversation

@davidlehn

Copy link
Copy Markdown
Member

No description provided.

@davidlehn
davidlehn requested review from applesnort and dlongley July 25, 2026 03:05
@codecov-commenter

codecov-commenter commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.27%. Comparing base (db99692) to head (08cde11).

Files with missing lines Patch % Lines
lib/httpClient.js 93.10% 2 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           update-deps      #52      +/-   ##
===============================================
- Coverage        94.05%   88.27%   -5.79%     
===============================================
  Files                4        3       -1     
  Lines              303      290      -13     
===============================================
- Hits               285      256      -29     
- Misses              18       34      +16     
Files with missing lines Coverage Δ
lib/agentCompatibility.js 81.74% <100.00%> (-11.54%) ⬇️
lib/index.js 100.00% <100.00%> (ø)
lib/httpClient.js 92.76% <93.10%> (-0.75%) ⬇️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db99692...08cde11. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dlongley dlongley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems ok from a glance -- hard to tell, without implementation experience, whether any new strange behavior is introduced.

davidlehn and others added 19 commits August 11, 2026 01:03
- 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>
Aligns the installed undici with the one built into the current Node.js LTS
release, per the policy of optimizing for current LTS.

undici 7 is the transition release between the old (v6 `onError`) and new
(v8 `onRequestStart`) dispatcher handler dialects, shipping both
`wrap-handler` and `unwrap-handler` to translate in either direction. A v7
dispatcher is therefore usable by the `fetch` built into Node.js 22, 24, and
26 alike, so the legacy `agent`/`httpsAgent` options now use the platform
`fetch` on every supported release and responses are platform `Response`
instances again.

Replace the strict installed-equals-platform major check with an explicit
table of the platform majors each installed major can drive. An installed
major with no entry falls back to requiring an exact match, so a missing or
stale entry costs only the fallback path rather than correctness.

The fallback is kept rather than removed: a future undici bump is expected
to need it again, since a v8 dispatcher cannot be driven by the v7 `fetch`
in Node.js 24.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidlehn
davidlehn merged commit 8590f56 into update-deps Aug 12, 2026
6 checks passed
@davidlehn
davidlehn deleted the remove-cjs-support branch August 12, 2026 00:01
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.

3 participants