Repository navigation
Update dependencies, remove CJS support, update supported platforms. #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
27d8bb1
f04db82
db99692
9e1a1b1
6c3f2d0
cde63c6
5a465d9
46f541c
a18c11e
2e4cec6
8998287
014ff19
6c3b445
ed16df9
95a3388
fd23c6d
2a8d574
83ed66c
8ab4c1d
47017b4
54f4eba
77f7154
d5bd6c8
18b3c4d
ae9dc47
4d0a36f
0279c3a
d7735eb
4df73c2
3062edc
54eeac2
b895f0a
acc3caf
2cea20b
842367f
5fdc05f
58a3f77
f4eb8ae
54ea253
b98212a
a63ad5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,13 +8,11 @@ import {versions} from 'node:process'; | |
| /* | ||
| Background: node ships its own copy of undici in the platform but does not | ||
| expose it (there is no `node:undici`), so this package installs its own. A | ||
| dispatcher only works with the undici that created it -- the handler contract | ||
| changed across majors, so handing an installed v6 dispatcher to a platform v7 | ||
| or v8 `fetch` fails with "invalid onError method". Which major the platform | ||
| provides varies by release line (node 22 has 6, node 24 has 7, node 26 has 8), | ||
| so no single installed version matches every supported runtime -- with undici 6 | ||
| installed, both node 24 and node 26 take the fallback path below. See | ||
| digitalbazaar/http-client#43. | ||
| dispatcher is only usable by an undici that speaks its handler dialect -- | ||
| that contract changed across majors, so a mismatched pairing fails with | ||
| "invalid onError method" or "invalid onRequestStart method". Which major the | ||
| platform provides varies by release line (node 22 has 6, node 24 has 7, node | ||
| 26 has 8). See digitalbazaar/http-client#43. | ||
| */ | ||
|
|
||
| // as long as an agent has a reference to it, its associated dispatcher will | ||
|
|
@@ -26,34 +24,52 @@ const DISPATCHER_CACHE = new WeakMap(); | |
| // its agent lives, so the override has the same lifetime as the agent | ||
| const FETCH_CACHE = new WeakMap(); | ||
|
|
||
| // can only convert agent to dispatcher option on node 18.2+ | ||
| const [major, minor] = versions.node.split('.').map(v => parseInt(v, 10)); | ||
| const canConvert = (major > 18) || (major === 18 && minor >= 2); | ||
| /* | ||
| Platform undici majors that each installed undici major's dispatcher can be | ||
| driven by. undici 7 is a transition release: it ships both `wrap-handler` and | ||
| `unwrap-handler` and translates between the old (v6 `onError`) and new (v8 | ||
| `onRequestStart`) handler dialects in both directions, so a v7 dispatcher | ||
| works with platform undici 6, 7, and 8 -- every node this package supports. | ||
|
|
||
| Revisit when bumping undici. An installed major that is not listed falls back | ||
| to requiring an exact match, so a missing or stale entry only costs the | ||
| fallback path below -- still correct, just not the platform `fetch`. | ||
| */ | ||
| const COMPATIBLE_PLATFORM_MAJORS = { | ||
| 7: [6, 7, 8] | ||
| }; | ||
|
|
||
| /* | ||
| True when the installed and platform undici majors match, meaning their | ||
| dispatchers are interchangeable. Both reads are guarded: a future undici could | ||
| hide `package.json` behind an `exports` map, and `versions.undici` may be | ||
| absent. Either way fall back to `false` and use the installed undici's own | ||
| fetch -- the always-safe path -- rather than throwing at module load and | ||
| breaking `import` for every consumer. | ||
| True when the installed undici's dispatcher can be handed to the platform | ||
| `fetch`. Both reads are guarded: a future undici could hide `package.json` | ||
| behind an `exports` map, and `versions.undici` may be absent. Either way fall | ||
| back to `false` and use the installed undici's own fetch -- the always-safe | ||
| path -- rather than throwing at module load and breaking `import` for every | ||
| consumer. | ||
|
|
||
| The explicit integer check matters: `parseInt` returns `NaN` instead of | ||
| throwing, so an unreadable version never reaches the `catch`. If both reads | ||
| were unreadable the lookup would fall back to `[NaN]`, and `includes` matches | ||
| `NaN` to `NaN` under SameValueZero -- reporting compatible, the opposite of | ||
| the safe default. | ||
| */ | ||
| const platformFetchCompatible = (() => { | ||
| try { | ||
| const installedMajor = parseInt(undiciPkg.version, 10); | ||
| const platformMajor = parseInt(versions.undici, 10); | ||
| return platformMajor === installedMajor; | ||
| } catch{ | ||
| if(!Number.isInteger(installedMajor) || !Number.isInteger(platformMajor)) { | ||
| return false; | ||
| } | ||
| const compatible = | ||
| COMPATIBLE_PLATFORM_MAJORS[installedMajor] ?? [installedMajor]; | ||
| return compatible.includes(platformMajor); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The guard fails open when both version reads fail, which is the one case the comment above it promises is safe. Three of the four cases are right. In the fourth, Both reads failing at once is unlikely, so this is low severity — but the fix is small: require
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| } catch { | ||
| return false; | ||
| } | ||
| })(); | ||
|
|
||
| // converts `agent`/`httpsAgent` option to a dispatcher option | ||
| export function convertAgent(options) { | ||
| if(!canConvert) { | ||
| return options; | ||
| } | ||
|
|
||
| // do not override custom fetch function from another lib | ||
| if(options?.fetch && !options.fetch._httpClientCustomFetch) { | ||
| return options; | ||
|
|
@@ -77,7 +93,7 @@ export function convertAgent(options) { | |
| delete rest.agent; | ||
| delete rest.httpsAgent; | ||
|
|
||
| // majors match: hand the dispatcher to `ky`, which forwards it to the | ||
| // compatible: hand the dispatcher to `ky`, which forwards it to the | ||
| // platform `fetch` (`ky` deliberately keeps `dispatcher` out of its | ||
| // request-option registry so it reaches fetch) -- no wrapper needed | ||
| if(platformFetchCompatible) { | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this might be the main tricky bit for upgraders.