Skip to content

chore: consolidate open Dependabot updates (typescript 6, undici 8) - #360

Merged
lv10 merged 4 commits into
masterfrom
chore/consolidate-dependabot
Aug 28, 2026
Merged

lv10 merged 4 commits into
masterfrom
chore/consolidate-dependabot

Conversation

@lv10

@lv10 lv10 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Consolidates both of the repository's open Dependabot pull requests into one verified change.

Closes #358

What is in here

Dependabot PR Package From → To
#272 typescript (devDep) 5.9.3 → 6.0.3
#347 undici (devDep) 7.29.0 → 8.10.0

Nothing is left out. Both bumps are devDependencies; dependencies (jsrsasign ^11.1.0) is
untouched and the package declares no peerDependencies, so the consumer contract of the published
package does not change.

Exactly two packages move in package-lock.json — node_modules/typescript and
node_modules/undici — both at versions (and integrity hashes) matching Dependabot's own branches.
Because the lockfile is lockfileVersion: 2 that shows up as five changed JSON entries: those two,
the root packages[""] devDependency specs, and the legacy dependencies.typescript /
dependencies.undici mirrors. The lockfile stays on lockfileVersion: 2, the entry count is
unchanged at 204, and nothing moved between hoisted and nested positions (compared path-keyed, not
name-keyed). A control npm install on an unmodified master checkout with the same npm produced
a zero-line lockfile diff, so none of the diff below is npm-version churn. No in-range drift
was picked up either: @commitlint/cli, @types/node, eslint, lint-staged and
typescript-eslint all have newer versions available and are deliberately left for Dependabot.

Both PRs have been red for a long time (#272 since April), and in neither case was the cause a
lockfile problem. Each needed a small, specific fix.


1. TypeScript 6 needs an explicit rootDir

typescript@6 fails the existing build outright:

tsconfig.cjs.json(4,5): error TS5011: The common source directory of 'tsconfig.cjs.json' is
'./src'. The 'rootDir' setting must be explicitly set to this or another path to adjust your
output's file layout.

TypeScript 6 no longer infers the common source directory when outDir is set, so tsconfig.json
now declares the ./src root the compiler previously computed.

Setting rootDir has one side effect worth calling out: it also changes where tsc -b computes
the default .tsbuildinfo path, moving both files from dist/cjs/ and dist/esm/ up to dist/.
Because package.json ships "files": ["dist"], that would have quietly changed the layout of the
published tarball. tsBuildInfoFile is therefore pinned to the original locations in
tsconfig.cjs.json / tsconfig.esm.json, in its own commit.

The published output is effectively unchanged. Built with TypeScript 6.0.3 versus 5.9.3 on
master, dist/ has an identical file set (npm pack --dry-run lists the same files at the same
sizes), and all eight emitted files — dist/esm/index.js, dist/esm/types.js,
dist/esm/index.d.ts, dist/esm/types.d.ts, dist/cjs/index.cjs, dist/cjs/types.cjs,
dist/cjs/index.d.cts, dist/cjs/types.d.cts — are byte-identical (cmp). The only difference
anywhere under dist/ is the compiler version string recorded inside the two incremental-build
files:

< {"root":["../../src/index.ts","../../src/types.ts"],"version":"5.9.3"}
> {"root":["../../src/index.ts","../../src/types.ts"],"version":"6.0.3"}

(Those .tsbuildinfo files being inside "files" at all is a pre-existing packaging wart; it is
left alone here.)

TypeScript 7 was evaluated and rejected

typescript@7.0.2 is the current latest. It actually compiles this project fine once rootDir
is set — but it cannot land yet, because typescript-eslint will not accept it. Its typescript
peer range is >=4.8.4 <6.1.0 on the current 8.68.0, and across all published versions the
upper bound has only ever been <5.8.0, <5.9.0, <6.0.0 or <6.1.0 (<6.1.0 first appears in
8.58.0); the canary tag is no different. Installing TypeScript 7 makes npm ls --all exit 1
with an invalid peer and forces the whole @typescript-eslint/* subtree to nest, growing the
lockfile from 204 to 224 entries. 6.0.3 — Dependabot's own target — is the highest version that
keeps the tree peer-clean.


2. undici 8 needs the tests to stop relying on Node's built-in fetch

src/index.ts calls Node's built-in global fetch, and the tests intercept it with
MockAgent + setGlobalDispatcher. undici 8 routes the legacy global dispatcher slot that Node's
bundled fetch reads through a Dispatcher1Wrapper; in 8.0.3 that wrapper started forcing
allowH2: false (nodejs/undici#4989), which collides with the `${origin}#http1-only` client
key Agent[kDispatch] gained in 8.0.2. MockAgent.get(origin) still registers its MockPool
under the plain origin, so the lookup misses, a real client is created, and the request escapes
to the public internet
— which is why the failure looks like a data bug
(SyntaxError: Unexpected token '<', "<!DOCTYPE "...) rather than a missing interceptor.

Measured locally on Node 24 with npm test and the unmodified test file:

undici result
7.29.0 pass
8.0.0 fail — separate, earlier bug: lib/global.js guarded the legacy-slot write with if (nodeMajor === 22)
8.0.1 pass
8.0.2 pass
8.0.3 fail — the allowH2 collision described above
8.10.0 fail

The fix here is two lines in test/index.test.ts: call undici's install() before the suite runs.
That routes the globals through undici's own implementation, which reads the unwrapped dispatcher
slot and is intercepted correctly. install() is public, documented and Stability: 2 - Stable
(docs/docs/api/GlobalInstallation.md, added in undici 7.11.0). It is version-neutral — the test
change passes on the currently pinned 7.29.0 as well, which is why it is a separate commit from the
bump.

Honest tradeoff: install() overwrites ten globals process-wide for the lifetime of the
process — fetch, Headers, Response, Request, FormData, WebSocket, CloseEvent,
ErrorEvent, MessageEvent, EventSource — and the suite therefore exercises userland undici's
fetch rather than Node's bundled one. Only fetch matters to this library today, but it is a
real reduction in fidelity and should be reverted once an undici 8.x release carries the upstream
fix: nodejs/undici#5648 was merged on 2026-08-05 and closed nodejs/undici#5036, but 8.10.0 was
published on 2026-08-03 and therefore predates it — and 8.10.0 still reproduces the bug locally.
#359 records the analysis and the follow-up.

agent.disableNetConnect() is also added so that an ordinary unmatched request fails as a
MockNotMatchedError instead of silently going to the network. It does not guard against the
dispatcher-level bypass above — with install() removed and disableNetConnect() left in place,
undici 8.10.0 still reaches the live host — and the code comment says so.

One thing to be aware of: undici 8's own engines.node is >=22.19.0, up from >=20.18.1 on
7.29.0. This repo declares no engines and .nvmrc is lts/*, so CI is unaffected, but a
contributor still on Node 20 will now get EBADENGINE on install.


Note on #272's age

#272 was opened on 2026-04-20 and last touched on 2026-05-25, so its branch predates four months of
master. Merging it would not have regressed anything — a merge only applies what the branch
changed, and it changed nothing outside the typescript lines — but its tree is far enough behind
that the bump was rebuilt from master here rather than rebased.


Verification

Run on Node 24.18.0 / npm 11.16.0 locally. CI resolves .nvmrc (lts/*) to the current Node 24
LTS — the most recent CI run on this repo used 24.19.0 — so this is the same major and the same
npm 11 line, not the shell default of Node 22 / npm 12.

Check master this branch
npm ci pass, lockfile untouched pass, lockfile untouched
npm run build pass pass
npm run lint pass pass
npm test 2/2 pass 2/2 pass
npm ls --all exit 0 exit 0
clean-tree npm install from git archive — exit 0, zero lockfile drift

That is the exact command sequence the Lint and test job runs. All four commits are also
independently green.

What the tests do and do not prove

This is a Node library — it calls Node's global fetch and is consumed from Node — not SuiteScript
running inside NetSuite's runtime, so the suite does exercise the real shipped code path. But it is
two tests, and they cover the request path narrowly. Mutation checks confirm what is genuinely
asserted: changing the request URL, changing the HTTP method, or disabling access-token reuse each
turns the suite red. Changing or removing the Authorization header does not — the suite
proves the token is minted once, not that it is ever sent.

Everything below the client — the actual NetSuite RESTlet contract, a real OAuth token exchange,
and jsrsasign's PS256 signing, which the tests mock out — is not exercised by CI, on this
branch or on master. For the TypeScript bump the stronger signal is the byte-identical dist/
comparison above; for the undici bump it is that those assertions pass against a genuinely
intercepted request, which master cannot do at all on 8.10.0.

lv10 added 4 commits August 27, 2026 20:43
TypeScript 6 requires an explicit rootDir when outDir is set (TS5011),
so tsconfig.json now declares the './src' root the compiler previously
inferred. Emitted dist/ (JS and .d.ts/.d.cts) is byte-identical to the
build produced by TypeScript 5.9.3.
Node's built-in fetch reaches the global dispatcher through a wrapper
that forces `allowH2: false`. From undici 8.0.3 the agent then looks up
an `<origin>#http1-only` client key that MockAgent never registers, so
the request escapes to the real network and the test fails with a JSON
parse error on live HTML rather than on a missed interceptor
(nodejs/undici#5036 -- fix merged, unreleased as of 8.10.0).

Calling `install()` puts undici's own fetch on globalThis, which reads
the unwrapped dispatcher slot and is intercepted correctly.
`disableNetConnect()` makes any future interception failure loud
instead of silently hitting the network. Both work on the currently
pinned undici 7.29.0, so this commit stands on its own.
Setting rootDir changed where tsc -b computes the default .tsbuildinfo
path, moving both files from dist/cjs/ and dist/esm/ up to dist/. Since
package.json ships "files": ["dist"], that silently changed the layout
of the published tarball. Pinning the paths keeps the published file set
byte-for-byte what it was; the only remaining delta is the compiler
version string recorded inside the two incremental-build files.

@SociableSteve SociableSteve left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving. Consolidating two long-red Dependabot branches into one branch built on current master, rather than rebasing either, is the right call here, and the body does the thing that makes a consolidation reviewable: it says what each half was, why each was red, and what was deliberately left out.

The check that carries the most weight is the emit comparison, and it holds. I confirmed independently that all eight files under dist/ are byte-identical to the base build, so a TypeScript 5 to 6 major produced no change in shipped output at all. That is what makes this a devDependency bump rather than a release, and it is the claim a green suite would not have proved on its own.

The one item I want to name plainly, because it is the substantive one and the body already does not hide it, is test/index.test.ts:32. Calling install() routes globalThis.fetch through undici's own implementation so MockAgent can intercept it, which means the suite no longer exercises the Node built-in fetch path that the library actually runs on in production. src/index.ts calls global fetch, so from this commit the tested path and the shipped path are different implementations.

I am not treating that as a blocker, for three reasons that are visible in the code rather than inferred. The underlying cause is a real upstream defect, not a shortcut: from undici 8.0.3 Node's fetch wrapper forces allowH2: false, the agent then looks for an <origin>#http1-only client key MockAgent never registers, and the request escapes to the live network. The alternative to the workaround is therefore not "keep testing the real path", it is "tests silently make real HTTP calls", which is strictly worse. The 21-line comment above the call states the mechanism, cites nodejs/undici#5036, and says the fix is merged but unreleased as of 8.10.0. And #359 is open and records the analysis and the follow-up, so it is tracked rather than absorbed. The agent.disableNetConnect() comment at line 17 is likewise careful to say what it does not cover, which is the kind of accuracy that stops a workaround from being mistaken for a guarantee later.

What I would ask is only that #359 stays live until undici ships the fix, since the cost of this workaround is invisible from the test output: the suite will keep passing whether or not the built-in path still works.

There is no prior review history on this PR, so nothing carries forward.

Three non-blocking notes, none of them defects:

  1. There is still no engines.node field, and undici@8.10.0 declares >=22.19.0. It is a devDependency, so nothing reaches consumers and no published constraint is wrong. The effect is on contributors: a fresh npm install on Node 20 or 22.x below 22.19 now warns rather than being caught, and this bump is what raised the floor. One "engines": { "node": ">=22.19.0" } would surface it at install time instead of at test time.

  2. The added tsBuildInfoFile pins in tsconfig.cjs.json:5 and tsconfig.esm.json:5 point inside dist/, and files is ["dist"], so the two .tsbuildinfo files ship to npm. I checked the published 1.2.0 tarball and they are already in it, both at 70 bytes, so this PR preserves existing behaviour rather than introducing it. Worth mentioning only because the pin is a new line in this diff, so this is the moment where pointing them outside dist/ would cost nothing.

  3. The body opens with "both of the repository's open Dependabot pull requests". #272 and #347 were closed about forty seconds after this PR was opened, so that sentence is out of date rather than wrong: it was accurate when written. Worth a small edit because this repo squash-merges, so the body becomes the commit message and the staleness becomes permanent.

On provenance: the builds, the emit comparison, npm ci, lint and the test runs were the review pass's executions. What I verified in my own hands is the head SHA, both tsconfig files against base, package.json at head and base, test/index.test.ts including the comment quoted above, undici@8.10.0's declared engines, the state of #359, #272 and #347, and the contents of the published tarball.

@lv10
lv10 merged commit cbfdc4c into master Aug 28, 2026
3 checks passed
@lv10
lv10 deleted the chore/consolidate-dependabot branch August 28, 2026 05:01
@lv10
lv10 deployed to production August 28, 2026 05:01 — with GitHub Actions Active
@github-actions github-actions Bot mentioned this pull request Aug 28, 2026

This branch was successfully deployed

1 active deployment
production — c26ac83f Deployed Aug 28, 2026 by lv10 via release #36
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.

chore: consolidate the open Dependabot updates into one PR MockAgent as global dispatcher broken since version 8.0.3

2 participants