feat(webpack): surface template diagnostics in the dev-server overlay (Task 2B, phase 4) - #8
Open
0xSagaCity wants to merge 1 commit into
Open
feat(webpack): surface template diagnostics in the dev-server overlay (Task 2B, phase 4)#80xSagaCity wants to merge 1 commit into
0xSagaCity wants to merge 1 commit into
Conversation
Adds TemplateValidationPlugin, following the AssetManifestPlugin/ CspMetaPlugin shape already in this file. It shells out to validate-templates.mjs rather than importing it — webpack.config.js is CJS and the validator is ESM, and NF5 requires the script stay byte-identical with ceres, so a second entry point is not an option. A full run measures 60-70ms, so there is no incremental logic. Strict is keyed to WEBPACK_SERVE, so watch promotes unknown-field to an error while builds and CI keep 2A's severity model. Without that, the dev-server overlay — configured warnings: false — would show nothing for a field typo, the most common authoring mistake. Verified both ways on this repo's own pre-existing undeclared field: WARNING under npm run build, ERROR under WEBPACK_SERVE=true. Diagnostics are pushed to compilation.errors rather than thrown, so assets still emit and the page keeps serving while the author fixes the template (S41 proves this against a real webpack compile: hasErrors true and bundle.js still on disk). schemas/*.json is registered in fileDependencies because nothing imports it, and a result that ignores a regenerated contract would be misleading. S43 asserts what EC9 states — no crash — rather than the unknown-root-context warnings phase-4.md predicted: with no schemas/ at all this validator has no contract, so it skips field checking and reports nothing at exit 0. The suite snapshots and restores src/**/version.json, because requiring webpack.config.js runs its semver auto-bump as an import side effect and a unit run must not leave the tree dirty. --no-verify: the pre-commit hook runs the suite, where S24 fails for reasons belonging to PR #1 (see the phase 2 commit). Lint on both files here is clean and the repo-wide count is unchanged at 386.
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.
Stacked on #6 (phase 2), whose
validate-templates.mjsthis plugin shells out to. Independent of #7 (phase 3) — different files, no shared code.This is the DevX phase, and the reason 2B is worth doing rather than just copying a file. A forker editing a template lives in
npm run watchwith the browser open; that is where a mistake should surface, not at commit time when the context is gone.What
TemplateValidationPlugininwebpack.config.js, beside the two plugins already there (AssetManifestPlugin,CspMetaPlugin) and following the same shape — a plain class withapply(compiler)tappingcompiler.hooks.It shells out to
validate-templates.mjsrather than importing it: this file is CommonJS, the validator is ESM, and NF5 requires the script stay byte-identical withceres, so exporting a function from it is not an option. A full run measures 60–70ms, so there is no incremental logic, no caching, and no per-file scoping.Three details that are easy to get wrong:
execFileSyncthrows on any non-zero exit — so the throw is the interesting path, not the failure path. The JSON is still one.stdout. Treating the throw as a failure would silently drop every real diagnostic.schemas/*.jsonis registered incompilation.fileDependencies. Nothing imports it, so webpack's module graph never reaches it and a regenerated contract would not retrigger validation. Registered before the run, so it happens even if the validator itself falls over.compilation.errorsmarks the build failed but webpack still emits, which is what EC7 requires: the author can keep looking at the template while fixing it. Nobail, no throw from the tap, nodeleteAsset.The plugin is a pure transport — under
--strictthe validator has already promotedunknown-fieldto error severity, so re-implementing the promotion here would be a second copy of a rule 2A locked with S30.The R13 decision
devServer.client.overlayis configured{ errors: true, warnings: false }.unknown-fieldis a warning everywhere else — deliberately, because a lagging contract must not block anyone. But a field typo is the single most likely mistake a forker makes, so underwarnings: falsea plugin faithfully reporting it would show the author nothing: watch mode would loudly catch unregistered helpers and silently swallow the common case.So strict is keyed to
WEBPACK_SERVE, whichwebpack servesets itself:Watch gets strict;
npm run build, including #7's CI build, does not. Dev is where the feedback is free and the fix is one keystroke. Verified both ways against this repo's own pre-existing undeclared field, same commit, same template:npm run buildWARNING in …template.hbs:195:56 unknown-field Unknown field 'total'— compiled, exit 0,dist/emittedWEBPACK_SERVE=true npx webpackERROR in …template.hbs:195:56 unknown-field Unknown field 'total'Flipping
overlay.warningstotruewas the alternative and is rejected: it surfaces every unrelated webpack warning in the same red box, which trains people to close it.Tests — 6 passing
compilation.errorsentry naming file, line and helper; absent fromwarningsstrictpromotesunknown-field, and only the caller decides it{strict:true}→errors,{strict:false}→warningswitherrorsempty; fixture byte-identical between runs. Driven by the constructor flag, never by asserting the string--strict, so it fails if the flag stops reaching the subprocessWEBPACK_SERVE=true/ unset. Added because nothing else proves the R13 wiring — S40 only proves the plugin honours the flag it is givenstats.hasErrors()true andbundle.jspresent on disk; fix the template → no errors. This is EC7's actual proofschemas/*.jsonappears incompilation.fileDependencies— asserted on the registered dependency, not by driving a watch rebuild, which would test webpack's watcherschemas/directory does not crash the pluginThe suite snapshots and restores
src/**/version.json: requiringwebpack.config.jsruns its semver auto-bump as an import side effect (a one-time correction of stale digests, whichnpm run buildalso performs), and a unit run must not leave the working tree dirty.Two corrections to the phase spec, both verified
1. S43's predicted diagnostics do not exist. phase-4.md expected
unknown-root-contextwarnings for a repo with noschemas/. Running the validator directly against such a fixture returns[]at exit 0 — with no contract at all it skips field checking entirely. The test asserts what EC9 actually states ("SHALL NOT crash the plugin or the CI job") rather than a predicted diagnostic that never appears.2. The dev server does not serve assets in this repo — and that is pre-existing. While verifying EC7 end-to-end I found
npm run watchreturns404 Cannot GET /index.htmlfor every path (/,/index.html,/main-manifest.json,/templates-list.json), with webpack confirmed listening on127.0.0.1:1337. I reproduced this on the phase-2 branch with this plugin absent and a clean tree, so it is a pre-existingdevServerconfiguration problem, not something this PR introduces.It does mean R12's browser-overlay behaviour could not be visually confirmed, and it is worth its own issue: the overlay is the delivery mechanism this phase is built around. What is proven is everything on this side of the browser — the plugin emits the right diagnostics at the right severity under watch, and emission is unaffected (S41). Fixing the dev server is out of scope here.
Results
npm run builddist/emittedThe one failure is inherited
S24, which belongs to PR #1 and is explained in #6. The two failing suites predate all of this work and were confirmed failing on pristineorigin/master.