Visitar URL original
chore: Update to Vitest 5 and move hook benchmarks to vitest bench (v6) by fratzinger · Pull Request #3722 · feathersjs/feathers · GitHub
Skip to content

chore: Update to Vitest 5 and move hook benchmarks to vitest bench (v6) - #3722

Merged
daffl merged 1 commit into
feathersjs:v6from
fratzinger:chore/vitest-5
Oct 28, 2026
Merged

daffl merged 1 commit into
feathersjs:v6from
fratzinger:chore/vitest-5

Conversation

@fratzinger

Copy link
Copy Markdown
Member

Summary

hooks/benchmark.test.ts fails randomly, and with Vitest 5 as the runner it fails almost every time. It asserts on a single Date.now() measurement:

  • the baseline is about 5ms, measured at 1ms resolution
  • the baseline is measured once in beforeAll, and the hooked functions are measured later, under different system load
  • it runs in parallel with the other 29 test files

This PR updates to Vitest 5 and moves the benchmark to Vitest 5's benchmark API. Benchmark files run in their own project, one at a time and never at the same time as tests. bench.compare() interleaves the iterations of the functions it compares.

 package.json
-  vitest ^3.2.4, @vitest/coverage-v8 ^3.2.4
+  vitest ^5.0.5, @vitest/coverage-v8 ^5.0.5, vite ^7.3.7
+  "bench": "vitest bench --run"
 packages/{feathers,create-feathers}/package.json
-  vitest ^5.0.0, @vitest/coverage-v8 ^5.0.0   (not used by `npm test`, which ran Vitest 3)
 packages/feathers/src/hooks/
-  benchmark.test.ts    Date.now() runtime < 20x baseline
+  benchmark.bench.ts   bench.compare(baseline, hooked): median (p50) overhead < 10x
 vitest.config.ts       benchmark.include, benchmarks excluded from coverage
 packages/feathers/tsconfig.json   benchmarks excluded from lib/ and cjs/
 .github/workflows/nodejs.yml      new "Run benchmarks (non-blocking)" step with continue-on-error
  • The assertion compares medians, because single GC pauses make the mean unreliable. Under load the mean ratio reached 52x with an rme of 114%.
  • The limit is back to the 10x from the original benchmark in feathersjs/hooks. It had been raised to 20x in v6.
  • Vite stays on v7. Vitest 5 allows Vite 8, but Vite 8's Oxc transformer only supports legacy (experimentalDecorators) decorators. Under Vite 8, decorator.test.ts, which uses standard @hooks decorators, fails with SyntaxError: Invalid or unexpected token.

No related issues, and no dependencies on other PRs.

Evidence

Measured as the ratio of hooked to baseline (p50), on Node.js 24.16 and 26.10:

empty hook single simple hook withParams and props
Node 24, no load (4 runs) 3.3 – 5.0 4.5 – 6.5 4.5 – 6.5
Node 26, no load (4 runs) 4.5 – 5.0 4.5 – 6.5 5.0 – 6.5
Node 24, running alongside the full test suite (5 runs) 4.5 – 5.0 5.0 – 6.9 3.7 – 6.5
  • Before: The old benchmark fails intermittently, for example with Runtime is 226ms, threshold is 160ms. With Vitest 5 it failed 2 out of 2 runs, with Runtime is 208ms, threshold is 180ms.
    After: It passed all 13 local runs, with a maximum ratio of 6.9 against the limit of 10. npm test passes on Node.js 24 and 26 with Test Files 29 passed, Tests 494 passed | 1 skipped. CI passes on 20.x and 24.x, and the benchmark step reports Tests 3 passed on both.

Merge Danger

Door: two-way

Blast Radius: small

  • This only affects dev tooling, and the benchmarks are no longer part of npm test. If the benchmark step fails, it shows up in the CI log but does not fail the build.
  • Vitest 5 declares node: ^22.12.0 || ^24.0.0 || >=26.0.0. It still runs on Node.js 20 (with EBADENGINE warnings); chore: Require Node.js 24 or later (v6) #3720 removes Node.js 20 from CI.
  • Vitest warns that the benchmarks "accessed module export getters too many times". The hooks implementation reads imported bindings through Vitest's module runner, which makes the hooked side slower than it would be in plain ESM, so the check is stricter, not looser. Running the benchmarks against the compiled lib/ would avoid that and could be done in a follow-up.
  • Projects that use Vite 8, Rolldown or Oxc can't use the standard @hooks decorators without an extra transform. This is worth mentioning in the v6 docs.

🤖 Generated with Claude Code

- Update to Vitest 5 in the root and remove the separate Vitest versions from
  the packages. Vite stays on v7 since the Oxc transformer in Vite 8 only
  supports legacy decorators and the tests use standard decorators
- The hook benchmarks were flaky because they compared single `Date.now()`
  measurements (a ~5ms baseline at 1ms resolution, measured at a different time
  than the hooked functions) while other test files ran in parallel. They now
  use `bench.compare()` which runs benchmark files on their own and interleaves
  the iterations, and assert the median overhead (at most 10x the plain function,
  as originally in feathersjs/hooks)
- Run the benchmarks with `npm run bench` as a non-blocking CI step and exclude
  them from the build and coverage

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fratzinger
fratzinger requested a review from daffl October 21, 2026 20:50
@daffl
daffl merged commit 51c6603 into feathersjs:v6 Oct 28, 2026
2 checks passed
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.

2 participants