feat: run on Bun and Deno, and prove it in CI - #352
Merged
Conversation
The shipped output had exactly one platform coupling left: `ts/assert-order/
StateMachine.ts` imported the bare `perf_hooks` specifier to reach a clock. That
was the only Node builtin anywhere in the published tarball. `node:assert` looks
like a second one from a source grep, but it appears only in `ts/testUtils.ts`
and the `.spec.ts` files, both excluded from `files` — it never ships, so there
is nothing to remove and no observable behaviour to break. Consumers already
catch this package's own `AssertionError` from `iso-error`, never Node's.
The clock now uses `performance.now()`, a global on Node >= 16, Bun, Deno and
the browser, with `Date.now()` as the fallback. The `process.hrtime` branch is
gone with it: it bought no precision `performance.now()` does not already give,
and it existed only because the module was written when the platform clock was
not yet universal. `browser: { "perf_hooks": false }` goes too — it stubbed an
import that no longer exists.
Biome's `useNodejsImportProtocol` is pinned to "error" so a bare `fs`/`path`
specifier cannot creep back in. The rule ships in the `style` group at `info`,
where biome reports the finding and still exits 0, so inheriting it from the
preset would have enforced nothing. Verified by reverting the import to its bare
form: `biome ci` then exits 1 with a real `lint/style/useNodejsImportProtocol`
error. The severity is set in `biome.json`, which is parsed as strict JSON — a
`//` comment there makes biome fall back to its defaults silently, so the
reasoning stays in this message.
`scripts/smoke.{mjs,cjs}` exercise the build output rather than `ts/`, because
the build output is all a consumer ever sees and a portability regression
surfaces there and nowhere else. The new `runtimes` workflow runs both against
Node, Bun and Deno on every pull request, all three fed the identical artifact
built once on Node. It is a separate job id from `code`, so the required
`code / all-checks` context is untouched, and `release` gains it as a `needs` so
a runtime regression fails the release closed the way `publish-gate` does.
Measured, not assumed: Deno 2.9 already resolves bare `perf_hooks` through its
Node compatibility layer, so the old code did in fact load there. The fix still
earns its place — it removes the builtin outright, which is what makes the
output loadable on the web and on edge runtimes that have no Node compat layer,
and it lets the lint rule guard the package with no suppression carved out of
it.
ESM-only is left alone deliberately: the package still ships a dual `esm/`+`cjs/`
build, and dropping the CJS half is a breaking change for its many dependents
that deserves its own PR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ETjy9oQGyETyFmDBdR9Egz
🦋 Changeset detectedLatest commit: 419f686 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #352 +/- ##
===========================================
+ Coverage 95.18% 100.00% +4.81%
===========================================
Files 21 21
Lines 187 179 -8
Branches 47 45 -2
===========================================
+ Hits 178 179 +1
+ Misses 8 0 -8
+ Partials 1 0 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`performance` is a guaranteed global on every host this package supports — `engines` requires Node >= 20, and Bun, Deno and browsers have had it far longer. The `Date.now()` arm was therefore dead code that no test could enter, which codecov correctly reported as an uncovered half of the diff. Deleting it rather than annotating it away is the honest fix: an untestable fallback is not a safety net, it is a second code path nobody has ever run. Coverage is now 100% on statements, branches, functions and lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ETjy9oQGyETyFmDBdR9Egz
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
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.
assertronis a leaf dependency of nearly every testing package in this fleet, so its portability ceiling is everyone else's. This raises it and, more importantly, puts a job in CI that executes the claim instead of asserting it in a table.What actually ships
Audited by packing the tarball and grepping the build output, not by grepping
ts/:perf_hooks(bare)ts/assert-order/StateMachine.tsesm/assert-order/StateMachine.jsand the CJSrequire("perf_hooks")node:assertts/testUtils.ts,ts/**/*.spec.tsfilesnode:fs/promisestsdown.config.tsSo the shipped coupling was one bare
perf_hooksimport, and one only.node:assertis not breaking, because it never shippedThe brief asked whether removing it breaks consumers. It does not, because there is nothing to remove:
node:assertappears only in the test helper and the spec files, andfilesexcludests/testUtils.tsand every*.spec.*.npm pack --dry-runconfirms neither is in the tarball. The library's ownAssertionErroralready comes fromiso-error(ts/errors.ts), never fromnode:assert— consumers catching an assertion failure have always been catching this package's error, with its own shape. No major changeset; this PR is a patch.The clock
StateMachinereached forperf_hooksand preferredprocess.hrtime. It now usesperformance.now(), a global on Node ≥20 (this package'senginesfloor), Bun, Deno and the browser. That is the same high-resolution millisecond clockprocess.hrtimewas providing, soAssertOrder#end()andgetTimeTaken()return what they always did. There is noDate.now()fallback arm: no supported host lacksperformance, and a branch no test can enter is not a safety net — codecov caught it as an uncovered half of the diff, and deleting it was the honest fix. Coverage is now 100% across statements, branches, functions and lines.browser: { "perf_hooks": false }is dropped along with it: it stubbed an import that no longer exists.The published output now references no Node builtin at all.
The measured result, including the part that undercuts the premise
Deno 2.9 already resolves bare
perf_hooksthrough its Node compatibility layer. I rebuilt the pre-change output and ran the smoke test on Deno to check rather than assume, and it passed. So the old code was not in fact broken on Deno, and this PR should not be read as a bug fix for it.It still earns its place. Removing the builtin outright is what makes the output loadable where there is no Node compat layer — browsers, workers, edge runtimes — and it clears the last suppression so the lint rule can guard the package with no carve-out.
Smoke results on the built output, ESM and CJS, all green:
Enforcement, verified rather than assumed
useNodejsImportProtocolis pinned to"error"inbiome.json. Inheriting it from the preset would have enforced nothing: the rule ships in thestylegroup atinfo, where biome prints the finding and still exits 0. Verified by reverting the import to its bare form —biome cithen exits 1 with a reallint/style/useNodejsImportProtocolerror — and by a scratch probe on a barepathimport. Restored afterwards.CI
.github/workflows/runtimes.ymlis a local reusable workflow running a three-way matrix. All three legs are handed the identical artifact, built once on Node with pnpm, so the matrix tests portability of the output and not of the build.pull-request.ymlcalls it as a separateruntimesjob. The caller job idcodeis untouched, so the requiredcode / all-checkscontext still exists.release.ymlcalls it and adds it torelease'sneeds, so a runtime regression fails the release closed, the waypublish-gatealready does.pnpm verifygains asmoketurbo task, so the Node leg also runs locally.Not done here
ESM-only: the package still ships dual
esm/+cjs/, and both halves are exercised by the smoke test. It could go ESM-only — nothing in the source needs CJS — but that is a breaking change for a package with this many dependents, and it deserves its own PR and its own major. Left alone deliberately.Checked and found already correct, so unchanged:
.husky/commit-msgis100755and commitlint ran on this commit;minimumReleaseAgealready lives inpnpm-workspace.yamlwithminimumReleaseAgeStrict: true, not in.npmrc; everyfilesentry resolves to a real directory. Workflow refs staycyberuni/.github/...— this repo lives incyberuni, notunional.🤖 Generated with Claude Code
https://claude.ai/code/session_01ETjy9oQGyETyFmDBdR9Egz