perf: validate options with a precompiled schema - #5746
Conversation
`schema-utils`'s `validate()` compiles the options schema with ajv on its first call. That cost 117ms of every `new Server()` — measured over eight interleaved runs, 117.3 +/- 2.8ms, against 0.07ms for each call after it. It is a one-time price every user pays on every start. `lib/options.check.js` is that schema precompiled by `ajv`'s standalone codegen. The constructor asks it first and only falls back to `schema-utils` when it rejects, so the happy path never loads ajv and an invalid config still gets the same message. webpack solves its own schema the same way, and the generated validator agrees with `schema-utils` on all 183 cases of the `validate-options` corpus plus 62 hand-written ones. The constructor drops to 1.0ms, and time-to-listening for a hello-world falls from 565 +/- 14ms to 523 +/- 25ms.
🦋 Changeset detectedLatest commit: 2bc55dd 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review. WalkthroughThe change adds an Ajv-based generator for a standalone options validator. Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`compiler.hooks.validate` and `compiler.validate`'s lazy-schema and precompiled-check parameters landed together in webpack 5.106, so the hook doubles as the feature probe for them. When it is there, validation goes through the compiler the way webpack's own plugins do, which also lets `validate: false` turn it off. The direct path stays as the fallback, with a TODO to drop it once the minimum supported webpack carries the hook. It is also what runs when no compiler was passed, which is how the server is constructed for `apply()`.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: e7f01b6c-f838-4d7d-bd65-776611ff907f
📒 Files selected for processing (1)
lib/Server.js
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
`compiler.validate` honours that compiler's own `validate` option, so asking the first child of a `MultiCompiler` read a policy that was never about these options: with `validate: false` on child 0 and the dev server configured on child 1, invalid options reached normalization instead of throwing. `getCompilerOptions` already had the selection rule — the child naming `devServer`, else the one targeting the web, else the first. It moves to `findDevServerCompiler`, which both callers now use so the two cannot drift apart again. Reported by CodeRabbit on #5746.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 3f7915a4-7eb6-4da2-b59f-5b5f8fd4fb6f
📒 Files selected for processing (2)
lib/Server.jstest/schema-check.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- test/schema-check.test.js
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5746 +/- ##
==========================================
- Coverage 90.83% 90.74% -0.09%
==========================================
Files 13 14 +1
Lines 6282 6329 +47
==========================================
+ Hits 5706 5743 +37
- Misses 576 586 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
codecov reported 6 uncovered lines in the diff, all of them fallbacks: `findDevServerCompiler` returning the first child when none names `devServer` or targets the web, and the direct `schema-utils` call taken when there is no compiler to ask, which is how the server is constructed for `apply()`. lib/Server.js's changed lines go from 70/76 to 76/76 covered.
Each `webpack()` call leaves a compiler holding a file-system cache and its purge timer. Node's test runner runs files concurrently, so those stayed alive alongside the browser suites for the rest of the run. Windows Node 22 timed out in `test/e2e/overlay.test.js` on the previous commit, which was test-only; that suite already uses most of its 400s there, and this is the load these tests were adding to it.
Used as a plugin the server is constructed without a compiler, so validating in the constructor had no `validate` policy to read and threw before `apply()` could bring the compiler the options are about. Validate in the constructor only when one is passed, and in `apply()` otherwise.
1 participant
Summary
schema-utils'svalidate()compiles the options schema with ajv on its first call, which cost 117ms of everynew Server()— measured over eight interleaved runs at 117.3 ± 2.8ms, against 0.07ms for each call after it. It is a one-time price every user pays on every start.lib/options.check.jsis that schema precompiled by ajv's standalone codegen, generated byscripts/generate-schema-check.mjsand verified current bylint:schema-check. The constructor asks it first and only falls back toschema-utilswhen it rejects, so the happy path never loads ajv and an invalid config still gets the same message — the same approach webpack uses for its own schema. The constructor drops to 1.0ms and time-to-listening for a hello-world falls from 565 ± 14ms to 523 ± 25ms.The generator refuses to emit if the schema grows a construct it would mistranslate (a
minLengthother than 1, an unsupportedinstanceof), so a future schema change fails generation rather than silently changing semantics.What kind of change does this PR introduce?
perf
Did you add tests for your changes?
Yes,
test/schema-check.test.js. The existingvalidate-optionssuite already catches a validator that wrongly accepts invalid options; it cannot see one that wrongly rejects valid options, becauseServerthen falls back toschema-utils, which accepts them — every test passes while the startup cost silently returns. Stubbing an always-false validator fails 17 of the new tests and zero of the existing 163. The generated validator was also checked to agree withschema-utilson all 183 cases of thevalidate-optionscorpus plus 62 hand-written ones, with no mismatches, and the 35 error-message snapshots are unchanged.Does this PR introduce a breaking change?
No. Validation accepts and rejects exactly what it did before, with identical error messages.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a — no public behaviour or option changes.
lib/options.check.jsis generated;npm run fix:schema-checkregenerates it andnpm run lint:schema-check(part oflint) fails if it is stale.Use of AI
AI-assisted (Claude Code). It was used to profile startup, to write the generator and tests, and to run the differential and A/B measurements. Every number quoted here came from a run, not an estimate: the arms were interleaved to absorb machine drift, and two other candidate optimisations were measured and discarded rather than shipped (parallelising the lazy
import()s showed no consistent win because module loading is compile-bound, and only ~14ms oflisten()is ours). All output was reviewed before committing.🤖 Generated with Claude Code
https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
Generated by Claude Code
Summary by CodeRabbit
Performance
Bug Fixes
Tests