feat: refresh page and layout edits in place in dev - #1405
Merged
Conversation
Every dev edit produced a full page reload: the page blinked, hydrated component state reset, scroll position was lost. Most dev edits do not need that. Pages and layouts never hydrate, so a freshly rendered page is the complete truth for them and nothing in the browser can be stale after a re-render. The dev watcher now classifies the changed file against the module graph and puts a ternary verdict on the SSE reload frame, and the browser picks the lightest correct response: morph the deepest shared boundary for a page edit (scroll and the hydrated state of components outside it survive), replace the whole body for a layout edit (its own markup sits outside every children range), and reload for anything else. The classification walks the graph rather than the path shape, because a page that imports a client-effecting util ships whole and a util under lib/utils reachable from a component is a component edit whose path says nothing. Everything unclassifiable reloads: a wrong morph is a broken page, a wrong reload is a flash. Component edits stay a full reload by design. customElements.define is once-per-tag, so a morph would apply new markup wired to the old class. This hot-swaps no module and does not reopen the deferred HMR question.
The relay's strongest-verdict rule is order-independent, so the burst test gets a counterfactual that a last-write-wins implementation fails. The refreshPage suite asserts what survives a swap against real DOM identity, a real custom-element upgrade, and real scroll, which is why it is a browser test rather than a string assertion.
The headline criteria are browser facts: whether the document reloaded is invisible in the DOM, so the fixture layout stamps a per-document token that survives a script re-run and can only change when the global scope is replaced. The component edit is the counterfactual in the direction that matters, since a morph there would wire fresh markup to the old class with no recovery short of a reload.
Collaborator
Author
|
Design rationale: why the verdict is ternary, and why the classifier walks the graph Two calls in here are worth writing down, because the diff shows what they are and not why. Why The honest cost of Why the classifier walks the module graph and never the path shape. "Anything under The input set is deliberately not What is deliberately conservative. A What I did not do. No prototype patching, no |
A new @webjsdev/core export has to be demoed or consciously exempted in the scaffold teaching-coverage manifest, and refreshPage earns a demo: re-rendering the page you are already on is a real capability an app can use after a mutation elsewhere, not just the seam the dev reload client calls. The page gains a server-rendered timestamp so the swap is visible, since the page function runs only on the server and restamps on every render.
Two defects in the same fallback path. refreshPage resolved true for anything that did not throw, and fetchAndApply throws for none of its real failures: a rejected fetch, a non-HTML body, an unparseable one, a discarded swap all return ok:false. So the dev client's full-reload fallback could never fire for the cases it exists to cover, and a tab would silently sit on stale content. It now reads the outcome. An aborted navigation is not a failure, since a newer navigation owns the page and reloading would yank the reader out of it. A swap also never re-requested the page's stylesheets: mergeHead preserves them unconditionally and the dev href carries no content hash, so the link node is kept by identity. A full reload used to do that asking, which is how webjs.dev.regenerate ever ran, since it rebuilds a stale output on request. Without it an edit that adds a utility class renders with no backing rule until a manual reload, which every in-repo app is configured for.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
I went through the whole change: the classifier ladder against the module graph, the SSE frame, the relay, and the refreshPage path down through performNavigation / fetchAndApply / applySwap.
The shape is right and the conservative calls are the ones I'd make. Two real problems, and they are the same problem twice: the fallback to a full reload is the safety net this whole feature rests on, and both halves of it were inert.
The first is that refreshPage can never report a failure, so the dev client's if (!ok) location.reload() is dead code. The second is subtler and worse in practice, because it fails silently on a working page: a swap never re-requests the stylesheets, and webjs.dev.regenerate only rebuilds a stale public/tailwind.css when someone asks for it. Every in-repo app is configured that way, so adding a utility class to a page would render it with no backing rule until a manual reload. That would have read as "the dev server is broken", not as "the morph skipped a step".
Nothing wrong with the classification itself, which is the part I expected to have to argue with.
Four problems from the delta round, three of them in the two fixes it was scoped to. The stylesheet refresh dropped the old link on error as well as on load, so a re-request that 404s (the server mid-restart, a renamed file) left the page with no stylesheet at all, which is the failure #936 and #1400 exist to prevent. On error it now drops the replacement and keeps the sheet that still works. Its de-dupe keyed on the path alone, so an author's second legitimately distinct link to the same file (a media=print sheet) was deleted on the first refresh and never came back. The key is now every attribute except the href query, which still collapses the bare/busted pair the head merge re-appends. refreshPage read the response's ok flag, which is the wrong question. An HTML body of any status is swapped in place, so a page rendered through notFound(), forbidden(), or an error boundary applied fine and was reported as a failure, making the dev client reload on top of a swap that already happened. fetchAndApply now reports applied-ness separately. The e2e comments also named the wrong mechanism: the fixture link is hoisted into head by the SSR, and the page tier merges through addNewHeadElements rather than mergeHead.
vivek7405
force-pushed
the
feat/dev-morph-page-edits
branch
from
August 13, 2026 12:36
7959d53 to
96b9c41
Compare
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Re-read the two fixes and traced what they touch. Three problems in them, and one wrong explanation in the tests.
The stylesheet refresh is the worrying one: it dropped the old link on error as well as on load, so a re-request that fails would leave the page with no stylesheet at all. That is the exact thing #936 and #1400 exist to prevent, and I introduced it while fixing a different instance of the same class.
The ok-versus-applied confusion is the interesting one. ok mirrors the HTTP status and an HTML body of ANY status is swapped in place, which is what makes the 422 revalidation and the error boundaries work at all. So reading ok reported a perfectly good swap of a 404 or 500 page as a failure, and the dev client would reload on top of it, losing the state the whole feature exists to keep, precisely when you are iterating on a broken page.
The load-versus-error rule was proven only by two regexes matched against the served client's source, which cannot tell a correct handler from one that removes the wrong node in a differently-formatted body. That is the half that leaves the page permanently unstyled, so it needs a real test. dev-styles.js follows the dev-overlay.js and dev-reload-worker.js pattern: a browser-safe module inlined verbatim into the served client after an export strip, so the browser test drives the exact shipping code. The new test asserts against real link elements and real load and error events, including the counterfactual that a 404 replacement removes itself and leaves the working sheet on the page. Three smaller things from the same round. performNavigation's returns tag did not declare the applied field its one consumer reads. The stream-action branch hardcoded applied true even when a newer navigation had superseded it, contradicting the contract written three lines above. And the public/ rung in the classifier still justified itself with "mergeHead preserves stylesheets so a swap would do nothing", which this PR made false; the rung stands, but the honest reason is that public/ is outside the module graph so the server cannot tell what the file is.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Read the two fixes again and traced where their concepts landed elsewhere. Nothing broken in what they do; five places where the code and its own stated contract had drifted apart.
The one that matters is the test coverage. The load-versus-error asymmetry is the half that decides whether a failed re-request leaves the page unstyled, and it was proven only by two regexes matched against the served client's source. Those pin formatting rather than behaviour: a reindent breaks them, and an implementation that removes the wrong node inside a differently-shaped handler passes them.
The rest is contract drift the fix itself introduced: a @returns that does not mention the field its one consumer reads, a hardcoded applied: true on the one return whose value is not derived from whether anything applied, and a justification in the classifier that this PR made false.
The stream branch was the only return in fetchAndApply that could produce ok true alongside aborted true, so a caller reading ok as "this response was not superseded" would have been wrong there and nowhere else. Every sibling abort return forces ok false, and the function's own contract says so. The HTTP status is still on the status field for anyone who wants it.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Read the extraction and everything it touches. One problem, and it is one I introduced in the previous fix rather than anything in the extraction itself.
The extraction checks out: same selector, same origin filter, same identity key, same load and error asymmetry, same buster. The relaxed served-client assertions still fail if the module stops shipping or stops being called, and the rung-6 rewrite is consistent across the classifier, its test, and the package AGENTS.md.
loadFrame passes the fetchAndApply outcome straight through, so it gained applied on its success path while its three guard returns and both its JSDoc and published .d.ts still described the three-field shape. That is the same contract hole the flag exists to close, left on the exported wrapper of the contract. The gallery teaching comment had the refreshPage paragraph spliced into the middle of a sentence, leaving a dangling clause and the same token twice. That comment ships as reference material in every scaffolded app. The runtime docs page told users to run the CLI directly while every other run instruction on it is an npm script.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Read the last fix and traced every consumer of the outcome object, plus the classifier ladder and the served client again. The fix itself is right and unobservable where it changes a value, which is what I wanted to confirm.
Three things left. The one that matters is that I closed the contract hole on fetchAndApply and left it open on loadFrame, which is the exported wrapper of the same object. The other two are prose I damaged: a teaching comment that ships in every scaffolded app, spliced mid-sentence, and a docs page telling users to run the CLI directly when every other instruction on it is an npm script.
…ed swap The feature turned itself off for every edit after the first in a burst. doRebuild invalidates the lazy analysis, nothing re-warms it until an HTTP request arrives, and the relay defers that request by its 2000ms quiet window while the measured inter-save gap is about a second. So the second save classified analysis-cold and the strongest-verdict rule collapsed the whole batch to a full reload. The gate now reads whether the derived sets are POPULATED rather than whether they are current, which is what the rest of the code already assumed: classifying against the previous build's graph is intended, and it is conservative in the right direction, since a file that graph has never seen falls through to a reload. applySwap returns without committing on four paths (a missing frame, and three degradations to a hard navigation), and all four still reported applied true, which is the hole the flag exists to close. They return a sentinel now and fetchAndApply maps it. Three doc corrections. The gallery copy described a counter that is not on that page. The skill reference kept the direct CLI invocation the website copy had already dropped. And both runtime surfaces listed five watched directories while the supervisor also watches root middleware, so a middleware edit is a full reload rather than the in-place refresh they implied.
The previous commit returned early on the four non-committing applySwap paths, which also skipped the history push, the scroll block, and the streaming tail those paths used to fall through to. A click-driven frame nav records history, so a frame-missing response would have stopped advancing the URL. Nothing covers that, so the absence of a red test was not evidence. It is a flag now, so only the reported value changes.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
The important one here is not a slip, it is the feature switching itself off.
A rebuild invalidates the lazy analysis, and nothing re-warms it until an HTTP request arrives. The relay defers that request by its 2000ms quiet window, and the measured inter-save gap is about a second. So on the second save of a burst the classifier was cold, returned analysis-cold, and the strongest-verdict rule collapsed the whole batch to a full reload. Every test warmed with a fetch before its single edit, so none of them could see it.
The gate was asking the wrong question. It read whether the derived sets are CURRENT, while the rest of the code already assumed the opposite and said so: classifying against the previous build's graph is intended, and it is conservative in the right direction, because a file that graph has never seen falls through to a reload anyway. It reads whether the sets are POPULATED now.
The rest is the applied flag still lying on four paths, and three doc claims that were wrong.
Collaborator
Author
|
State: the last two commits are unreviewed, and the PR stays a draft until they are Recording where this stands so a cold reader does not have to infer it. The change itself has been read five times over. What is NOT covered is the last two commits, I believe that is right, and the reasoning was already written into
Worth noting what the rounds actually turned up, because the shape is consistent. Two of them found defects in the ORIGINAL change. The other three found defects in the fixes for the previous round, and the majority of those were the same kind of mistake: a fix that closed a hole in one place and left the identical hole in the place next to it (the |
Both the sentinel and the flag that reads it shipped with no assertion: the only applied:false cases already returned false before the change, so they passed identically with it reverted. Three cases close that. A frame-missing loadFrame reports applied:false while a matching one reports true, a refresh that degrades to a hard navigation reports it did not apply, and a click-driven frame nav still advances the URL, which is the fall-through the flag exists to preserve and which nothing else in the suite would notice losing.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Read the two commits that landed after the last round, and traced the state machine and the sentinel across everything they touch.
The gate change holds up, and I checked the parts that could have made it unsafe rather than taking the reasoning on trust. doRebuild never clears the derived sets, so reading them stale is reading the previous build rather than reading nothing; seenFilesFor excludes public/, so a file the stale graph never saw does fall through to the reload rung. The new burst test is a real counterfactual, and it works because /__webjs/events is answered in the listener shell before ensureReady, so the second SSE connect cannot accidentally re-warm the analysis and mask the case.
One problem, and it is the one the previous commit's own message pointed at without closing.
vivek7405
marked this pull request as ready for review
August 13, 2026 14:49
vivek7405
marked this pull request as draft
August 13, 2026 14:57
It asserted the URL reached the link's target, but an earlier case in the same suite clicks the same link and leaves it there, and web-test-runner isolates per file rather than per test. So the assertion was satisfied by that case's history push and would have passed with this one's removed. It parks the URL somewhere the click cannot reach first. Proven by deleting the earlier case and re-running: the assertion still fails under the early-return shape and passes under the fix.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Checked the test-only commit, specifically whether the three new cases observe what they claim.
Two of them do: reverting the 'none' sentinel genuinely reds the loadFrame applied case and the refresh-degradation case.
The third does not, and it is the one I was most confident about.
The parking replaceState and its assertion sat between setup() and the try, the one point in the file where a throw skips the finally. A failed park, or WebKit rate-limiting history mutations, would have leaked the parked URL, the patched console, the nav guard and the container into the three later cases, which would then fail against the wrong container for reasons naming nothing about the cause. That is the cross-case state leak this case exists to close. Proven by injecting a throw at the park step: exactly one case fails, the one that threw, with no cascade.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
Checked whether the parking actually isolates the case, and whether it disturbed anything else in the file.
It does isolate it: onClick has no same-URL short circuit, the frame-missing branch returns the sentinel, and fetchAndApply still falls through to the history push, so the assertion observes this click. Nothing else in the file moved: loadFrame passes recordHistory: false so the preceding case cannot shift the URL, snapshotCurrent keys on the internal page url rather than location, and the later cases see the same URL they always did.
One problem, and it is the same class as the bug being fixed.
Reading `location` to prove a history push needed two mutations of its own, a park before and a restore after. Both were hazards: this file's only cross-case leaks came from exactly those two lines, and WebKit rate -limits history mutations so either can throw and strand shared page state on the cases below. The last fix moved the park inside the try and left the restore, in the same finally, doing the same thing. Spying on history.pushState needs neither mutation, depends on no sibling case, and asserts the call directly rather than inferring it from a global the whole file shares. Proven by deleting every other case in the file: the spy case alone still passes under the fix and fails under the early-return shape. Also drops a no-op in refresh-page.test.js whose comment claimed to restore router state for "the rest of the file", in the last test of the suite, after the finally had already left it disabled.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
The move does what it says, and it did not change what the case observes or disturb anything else in the file. But it fixed one of the two history mutations in that case and left the other, in the same finally, exposed to the same throw the new comment names.
That is the fourth finding in a row in this one file, all of them about the scaffolding rather than the behaviour, so I stopped patching the ordering and removed the need for the mutations instead.
The overlay's teardown is keyed on the URL CHANGING, and a same-url refresh never changes it, so a refresh has no exit for a live overlay the way a full reload did by replacing the document. Shipping this as defensive, because the scenario that looks like it needs it does not hold, and that is worth recording so the next reader does not re-derive it. A page-render error cannot strand an overlay: the dev 500 page carries no children boundaries and no layout, so applying it shares no boundary with the live page and degrades to a hard navigation in BOTH directions, and the reload clears the overlay. Measured, not assumed. What remains is an UNSCOPED frame (a rebuild or ts-strip failure), which the nav sync deliberately never clears and which can coexist with a page that still renders and therefore still refreshes. Dismissing before rather than after is self-correcting: a render that fails again pushes a fresh frame during it, so the overlay returns describing the current error. Afterwards would race that push. No e2e for it. I wrote one, found it passed because the break had already hard-navigated rather than because of the dismiss, and removed it rather than ship a test that observes the wrong thing.
vivek7405
commented
Aug 13, 2026
vivek7405
left a comment
Collaborator
Author
There was a problem hiding this comment.
The spy restructure itself checks out: it observes the push from its own click, cannot be satisfied by another case, and swallowing the push is inert for the rest of the file.
The interesting thing came out of the blast radius rather than the commit.
vivek7405
marked this pull request as ready for review
August 13, 2026 20:43
vivek7405
added a commit
that referenced
this pull request
Aug 13, 2026
#1405 adds in-place page and layout refresh to the dev live-reload path, which touches the two files this branch restructured hardest, so its changes had to be ported rather than merged. The two new modules (dev-classify.js, dev-styles.js) merged as new files. router-client.js: 19 of 23 hunks applied by content match. The four that did not are the refresh entry point being published on the global in enableClientRouter, and the `refresh` / `opts` parameter threading through performNavigation, fetchAndApply and applySwap. Ported by hand from main, with the barrel gaining the new `refreshPage` export that `packages/core/index.js` re-exports. dev.js: 15 of 18 hunks applied. Hand-ported the dev-classify import, the `analysisEverDone` flag, and DEV_STYLES_SRC, the last into dev/helpers.js where the other two inlined-source constants already live. Two things caught by the guards rather than by review. The free-identifier test found `strongerVerdict` imported into handler.js while server.js is what calls it. And splicing applySwap's new JSDoc in swallowed the `_swapCommit` declaration above it, because the branch had no doc directly over applySwap and the search ran back to the previous block; the barrel export check failed immediately. packages/server/AGENTS.md takes main's row, which carries #1405's content, with this branch's barrel sentence re-applied.
vivek7405
added a commit
that referenced
this pull request
Aug 14, 2026
Moving a module a directory deeper breaks its relative type references
as surely as its runtime imports, but only the runtime ones fail loudly.
A JSDoc `import('./x.js')` that no longer resolves degrades the annotated
symbol to an unresolved type in silence, and `packages/` has no tsconfig,
so nothing in CI looks.
21 of them across seven modules, including the whole `ReloadVerdict`
contract #1405 threads through `onReload`, `rebuild`, `doRebuild`,
`classifyWatchPath` and `pendingVerdict`, whose runtime import the merge
corrected while leaving the five type references pointing at
`dev/dev-classify.js`.
They were surfacing one review round at a time, so this adds the check
that finds them as a class. It skips the three specifiers that appear in
prose as illustrations of what an app author would write, matched
exactly so a real reference cannot hide behind one. Proven by pointing
one back at the wrong path and watching it fail.
vivek7405
added a commit
that referenced
this pull request
Aug 14, 2026
The head one matters most and I had claimed the opposite. An earlier comparison of rendered bytes between main and this branch reported "identical apart from a clock"; that comparison ran against a HYBRID tree, because `git checkout origin/main -- packages/` restores tracked files without deleting the branch-only ones, so it proved nothing. Redone against a clean worktree of main, the served `<head>` differs: main emits seven modulepreload hints, `@webjsdev/core` among them, BEFORE the icon / apple-touch-icon / canonical links, and this branch emitted them after, because the split moved the preload block to the end of `wrapHead`. That is a boot-critical hint-discovery change on every page. The block is back where main has it, verified by re-rendering and diffing the tag order. Five more, each a rewrite the split made while moving code: `<link rel="author">` was pushed to `linkTags` rather than `metaTags`, which the document template joins before `<title>`, so it moved in the head for any app declaring `metadata.authors[].url`. `cachedHtmlResponse` grew fallbacks main does not have, and one of them, `rec.body || rec`, puts the RECORD object in the response body for a cached record with an empty-string body, contradicting its own `@param`. There is one call site and it always passes a well-formed record, so the fallbacks bought nothing. The base-path-miss 404 lost its `content-type: text/plain`. The framework probes were matched against the DECODED path, so `/__webjs%2Fhealth` answered the liveness probe; they match the raw pathname again, guarded, as main does. `/__webjs/reload.js` and `/__webjs/reload-worker.js` were gated on `dev &&`, so in production they fell through the whole pipeline instead of returning the explicit 404 main returns, and they had gained a `cache-control` header main does not send. Also raises the router-client barrel floor to 69, which the #1405 merge left at 68 by adding `refreshPage` without it, and adds a test that every floor EQUALS its export count. A floor below the count tolerates losing exactly that many exports, which is the regression the guard exists to catch.
vivek7405
added a commit
that referenced
this pull request
Aug 14, 2026
…SOLID, KISS, and DRY principles (#1376) * test(architecture): add barrel surface export count guard for framework refactor * refactor(cli): barrel doctor.js into modular sub-modules Refactor packages/cli/lib/doctor.js into sub-modules under packages/cli/lib/doctor/ (codes, policy, util, manifest, route-modules, runner, probes/*). Preserves runtime export surface byte-identically and enforces minimum export count floor in test/architecture/barrel-surface.test.mjs. * refactor(server): barrel vendor.js into modular sub-modules * refactor(server): barrel check.js into modular sub-modules * refactor(server): barrel dev.js into modular sub-modules * refactor(server): barrel ssr.js into modular sub-modules * fix(server): restore complete metadata and streaming features in ssr barrel * refactor(core): barrel component.js into modular sub-modules * refactor(core): barrel render-client.js into modular sub-modules * refactor(core): barrel render-server.js into modular sub-modules * fix(server): restore behaviour the barrel splits silently changed The splits were largely faithful moves, but seven regions were rewritten rather than moved, and the rewrites changed observable behaviour that the export-surface guard cannot see, because the export NAMES all still match. 31 tests across packages/server/test caught it. * locateCoreDir resolved its workspace fallback relative to import.meta.url. The file moved one directory deeper, so the walk to packages/ needed four steps rather than three and landed on packages/server/core, which does not exist. Every /__webjs/core/* request 404d while the importmap still pointed at it. * dev/server.js referenced attachWebSocket without importing it, so every startServer call threw a ReferenceError. * dev/handler.js referenced applyTrailingSlash and withAssetHash without importing them. * The /__webjs/health and /__webjs/ready probes lost their no-store headers and their response shapes, and /__webjs/ready no longer kicked off the background warm. * The top-level middleware wrapper was dropped from the request path, so middleware.js never ran and a throwing middleware never became a 500. * The structured access log renamed durationMs to ms and changed its rounding, breaking the observability contract. * The SSR head template made the csp-nonce meta conditional, so a CSP-off document came out one newline shorter than a CSP-on one, and the default title changed. The template is now spliced back verbatim from the pre-split source. render-server.js also stopped re-exporting injectDSD. Nothing outside render-server/ imports it and render-server.d.ts does not declare it, so re-exporting widened the published @webjsdev/core/server surface for a helper no consumer asked for. The sigil-coverage guard read render-client.js and render-server.js directly to prove the renderers route binding recognition through the shared BINDING_PREFIXES. Those are barrels now, so it reads the barrel plus every module beneath it; scanning only the barrel would have made the guard vacuous. * test(cli): point the middleware-extension guard at the split dev tree The guard reads the server source and greps for ROOT_MIDDLEWARE_FILES so the watched extension list and the loaded one cannot drift apart. dev.js is a barrel over dev/ now, and the declaration moved into dev/handler.js, so the grep found nothing and the test failed on its own precondition. Read the barrel plus every module beneath it. Had the assertion been written the other way round it would have passed vacuously instead, which is the worse failure for a drift guard. * refactor(core): barrel slot.js into modular sub-modules Splits the 2282-line light-DOM slot runtime into seven modules under packages/core/src/slot/, leaving slot.js as a barrel over the same 31 public exports. Largest module is interception.js at 570 lines. symbols symbol keys, attribute names, shared constants polyfills native API capture and the light-DOM implementations state per-host state, authored capture, SSR adoption sensors MutationObserver sensors and the renderer backstop interception native insertion API interception on a slotted host project projection into slots and post-render resync assignment assignment commit, fallback restore, slotchange Every line moved VERBATIM. The only edits are the `export ` prefix where a declaration now crosses a module boundary, and the generated import lines. Verified two ways: the barrel's runtime export set is identical to the pre-split module's (31 names, none missing, none added), and every code line of the original survives byte-identical in the split tree once comments and that `export ` prefix are normalized away. Two placements are forced rather than chosen, both because an ESM import binding cannot be assigned across a module boundary. `inBrowser` sits with polyfills because installSlotPolyfills reassigns it, and the N_* natives sit with sensors because captureNatives assigns them. slot.js keeps its path rather than moving under component/, as the issue originally proposed: render-client.js, router-client.js and component.js all import it, so it is not a component-private concern, and filing it under component/ would invert the dependency. * fix(core): stamp forwarded slots with the real SLOT_OWNER symbol The render-client split swapped two imported symbols for Symbol.for() lookups: SLOT_STATE became Symbol.for('webjs.slotState') and SLOT_OWNER became Symbol.for('webjs.slotOwner'). slot.js creates both with Symbol(), which mints a unique value, not Symbol.for(), which interns one in the global registry under a string key. So the two lookups produced symbols no host has ever carried. ownerHost evaluated to null on every render, the stamp never landed, and a forwarded slot fell back to the structural parent walk, which picks the nested child rather than the host whose template produced the slot. That is the whole of #1023, and it broke silently: no node test covers it, the export surface is unchanged, and SSR bytes are unchanged, because the defect is entirely post-hydration. The three browser tests in packages/core/test/slots/browser/router-slot-architecture.test.js are the only thing that catches it, and they were red on this branch while green on main. * refactor(core): barrel router-client.js into modular sub-modules Splits the 5400-line client router into twenty modules under packages/core/src/router-client/, leaving router-client.js as a barrel over the same 68 exports (5 public entry points plus the 63 underscore test seams, whose names and aliases are unchanged). Largest module is dom-differ.js at 711 lines. constants dom-parse scroll upgrade state form-encoder frames diagnostics boundaries snapshot-cache prefetch nav-error fetch-apply view-transition swap dom-differ head-merge stream navigator events 1459 of the original 1460 code lines are byte-identical after the move. The single changed line is `const myToken = ++currentNavigationToken`, which became a bumpNavToken() call for the reason below. Three module-scope bindings are written from two modules each, and ESM forbids assigning an imported binding, so each owning module now exposes a one-statement accessor the navigator calls instead: restoreGeneration -> bumpRestoreGeneration() (scroll) currentNavigationToken -> bumpNavToken() (state) prefetchViewObserver -> teardownPrefetchViewObserver() (prefetch) `restoreGeneration` is still imported read-only alongside its accessor, because the deferred scroll restore captures it and re-compares after the frame. Dropping it from the import list left a free variable that threw inside the deferred callback, which showed up as a Back restore silently landing at offset 0 rather than as any node-side failure. Only the two #1310 browser tests caught it. Placement of mutable state is forced by its writers, not chosen: `enabled`, `activeAbortController`, `currentPageUrl` and `prevScrollRestoration` sit with the navigator because enableClientRouter, disableClientRouter, performNavigation, performSubmission and loadFrame are what write them. The `_setHardNavigate`, `_navToken`, `_bumpNavToken`, `_currentPageUrl` and `_setCurrentPageUrl` seams moved beside the state they write, for the same ESM reason, and the barrel re-exports them under their existing names so no test import changes. * test(server): point the enctype drift guard at the split router tree The guard pins three hardcoded copies of the text/plain denylist keyword against each other, and reads router-client.js off disk to check the client half. That file is a barrel over router-client/ now and the client guard lives in form-encoder.js, so the first assertion went red and the second (a doesNotMatch) would have passed vacuously. Read the barrel plus every module beneath it, which is what the guard means by "the client half". * docs(agents): record module-size and barrel-split guidance Nothing in the repo stated how big a source module may be, or how to split one safely, so this refactor had to settle both and the answers lived only in the issue. Size: target 800 lines, around 1000 at the most, barrels exempt, no CI guard. The number comes from measuring the clones this project takes its cues from, where lit-html.ts is 2303 lines, reactive-element.ts is 1754 and Vite's server/index.ts is 1447. All of them draw seams by responsibility and let the orchestration entry stay large, which is why a line-count gate is the wrong instrument. SOLID, DRY and KISS go in as prose judgment, outside webjs check, matching how the project already separates conventions from correctness rules. Splitting: a split is a MOVE, not a rewrite, verified by an export-set diff in both directions and a byte-identical code-line diff. The rest is the failure modes this refactor actually hit, every one of which was silent: mutable state must live with its writers because ESM forbids assigning an imported binding, a binding that is also read must stay in the import list, Symbol() and Symbol.for() are not interchangeable, a relative import.meta.url walk breaks when a file moves deeper, drift guards that read a source path start passing VACUOUSLY once that file becomes a barrel, dist must be rebuilt before e2e or Bun, and the browser suite is mandatory because these defects are post-hydration and leave both the export surface and the SSR bytes unchanged. Lands on all four surfaces: the cross-agent AGENTS.md, a new skill reference, framework-dev.md for the monorepo mechanics, and the core and server package files for what is specific to each. * docs(packages): restore the comments the barrel splits stripped The earlier splits moved most code faithfully but dropped roughly 7,700 lines of JSDoc and inline commentary along the way. That matters more here than in most codebases: WebJs ships buildless, the source IS what runs and what an agent greps, and AGENTS.md points readers straight at these files. Restored mechanically rather than by hand. For every top-level declaration in a split module, the pre-split declaration is taken from origin/main and, when the two are identical once comments and whitespace are normalized away, main's text replaces the split text verbatim. That recovers the doc block and every inline comment together, and it cannot change behaviour because it only fires where the code already matched. Proof it is comments-only: the normalized code of all ten split trees is byte-identical before and after, 12322 lines either way. 371 declarations restored, +5727 lines. 79 are left untouched because their code genuinely differs from main. Some of those are deliberate (the three router accessors, the dev and ssr behaviour restorations earlier on this branch), and the rest are places the earlier splits rewrote rather than moved. Those want reading by a human, not a script, so they keep whatever comments they have. A first attempt spliced blocks that had swallowed a following import statement and silently dropped 17 code lines. The script now refuses any block carrying a statement main does not have, and preserves the trailing blank-line separator. * fix(server): re-apply the icon metadata-route auto-link into the split #1379 landed on main after this branch was cut, adding setMetadataIconRoutes / autoMetadataRouteIcons to ssr.js and two call sites to dev.js. Both files are barrels here, so the rebase resolved those conflicts in favour of the barrel and the feature would otherwise have been REVERTED by merging this branch. Ported into the split rather than restored to the barrel: * `_metadataIconRoutes`, `setMetadataIconRoutes` and `autoMetadataRouteIcons` go in ssr/head.js, not beside `_clientRouterEnabled` in ssr/render.js where the pre-split file happened to keep them. `wrapHead` is the only reader and the only writer path runs through the setter, so the state belongs with the code that uses it. * ssr.js re-exports `setMetadataIconRoutes`, since dev/handler.js imports it through the barrel. * dev/handler.js binds it at boot and re-binds in doRebuild, so adding or deleting app/icon.* still takes effect without a restart. Verified against the feature's own tests rather than by inspection: 140 SSR tests, the two repo-health favicon suites, and test/bun/ metadata-icon-routes.mjs under Bun, which covers auto-link, the declared-icons precedence rule, and the basePath prefix. * fix: clear the removed rule's premise from the split tree The rebase onto #1385 dropped submitter-needs-bound-form from check/, but three comments still carried its premise: the render-client reconciler said a submitter asks whether its enclosing form is bound, the DSD pass still documented the 'unknown' form scope it no longer passes, and the check runner named the rule as a sharer of classifyActionHole. * docs(packages): restore the JSDoc the barrel splits dropped The splits moved code without its documentation: 120 JSDoc blocks present in the pre-split modules appeared nowhere in the sibling trees. Re-attach each surviving block to the symbol it documents, and fold the two module headers (check, vendor) back into their barrels alongside the barrel note. Left out on purpose: teardownUntil's block, whose function was already dead code on main (teardownChild inlines the abort), and the one-line inline `@type` casts, which sit inside expressions where an automated insert is not safe. * fix(server): restore dev live-reload the dev.js split broke Two defects, both silent. handler.js calls isRegenerateOutputPath without importing it (the split left the import in server.js, which never used it), so the first watch event threw a ReferenceError that the watcher's own catch reported as 'file watcher exited' and swallowed. server.js also imported watch from node:fs, whose callback API is not async-iterable, so the for-await over it could not work either. Live reload was dead in dev: no rebuild on any edit, in-tree or under a webjs.dev.watch root. * docs(cli): restore runDoctorChecks' JSDoc The block documents the two test-injection seams (nodeVersion, vendor) that nothing else describes. The split left the DoctorResult typedef sitting where the doc used to be, which is why the earlier sweep read it as documented. * fix(server): restore the basePath rebuild's spoofed-IP strip (#756) The dev.js split rebuilt the Request for a basePath app without deleting the inbound x-webjs-remote-ip header, and called propagateTrustedRemoteIp with the Headers object instead of the new Request, so the WeakMap entry was keyed to something no one reads. A client-supplied IP therefore survived the rebuild and won, which is exactly what #756 closed. The same rebuild also dropped redirect and signal, so an action under a basePath could not observe a client abort (#492). * fix(core,server): restore the types and the guard the splits weakened Finishing the JSDoc audit turned up three things a comment-level sweep hides, because the split kept a comment in place but changed what it said. Five casts in render-client were widened to `any`: TemplateInstance in three places, the repeat map's value type, and the array state's item type. The template compiler's formActions was widened from FormActionRecord[] to any[]. ssr's normalizeHint was rewritten as `typeof h === 'object' && h.url`, which accepts a non-string url where main required `typeof … === 'string'`. A hint whose url is a number or an object now reaches the head and is stringified into a link href. Restored main's guard. Also re-attached the remaining documentation: the dev version memo's type, and normalizeHint's signature doc with its parameter renamed to match the split. * refactor(core): break the render-server cycle and split dsd.js dsd.js and template-renderer.js imported each other. The back edge was weaker than it looked: template-renderer took four names from dsd.js and used only two of them, isRawtextTag and kebabCase, both pure string helpers. injectDSD and decodeAttrEntities were dead imports left over from the monolith. So the cycle breaks by extracting the leaves rather than by inverting anything. dsd.js loses its scanning primitives to html-scan.js, its name-case and entity decoding to text.js, its instance-facing attribute plumbing to attrs.js, and its light-DOM slot projection to slots.js, keeping the element walk and the suspense pass. template-renderer.js then takes its two helpers from the leaves and no longer reaches into dsd.js at all. That also lands dsd.js at 445 lines, under the plan's 1000 ceiling, so the two acceptance criteria are one change here rather than two. Drops dsd.js's entire form-action import block along the way: every one of those fifteen names is used in template-renderer.js and none in dsd.js. Dead imports are what let this cycle hide, so they are worth removing rather than carrying. * refactor(server): break the ssr cycle and split render.js document.js, head.js and render.js were mutually reachable through two back edges, both of which were misplaced state rather than real coupling. publicEnvShim lived in document.js and nothing in document.js used it, while head.js and render.js both did, so the import was on the file rather than on the code. The client-router flag had the same shape in reverse: render.js owned the module-level switch and head.js reached back for the reader. Both move to their own leaves, which leaves head <- document <- render one-way. render.js also carried a SECOND copy of wrapHead, 247 lines of it, which nothing called. main has exactly one wrapHead; the split produced two, wired document.js to head.js's copy, and left this one orphaned. The two had already drifted: head.js's handles the archives, assets and bookmarks link rels and metadata.other, and escapes module URLs through jsonForScriptTag rather than into single quotes. Deleting the dead copy is what makes the difference unable to matter later, and it orphaned every head-building import in render.js, which is the proof it was self-contained. That left render.js at exactly 1000 lines, which meets the ceiling with no headroom at all, so the preload computation moves to preloads.js. It is pure module-graph work that touches no request, response, or rendering, and it takes render.js to 821. * refactor: drop the dead imports the splits left behind 64 import bindings across the ten split trees name something the module never uses. They are not cosmetic: an import is a graph edge whether or not the binding is read, so these were holding the module graph in cycles that the code itself does not have. Removing them takes the cyclic components from four to two and the modules inside them from 31 to 17. router-client alone drops from 19 modules to 11, because constants.js was importing seven names from four different modules and using none of them, which made the directory's intended leaf a hub. Found by comparing each imported name against the module body with comment spans stripped. Two subtleties made that worth automating rather than eyeballing. A name can appear a dozen times in prose and never in code, which is most of these. And the codebase's inline cast idiom, `/** @type {any} */ (host)[SLOT_STATE]`, starts a line with `/**` while being ordinary code, so a line-shape reading of it drops a live import and yields a ReferenceError; SLOT_STATE, LIGHT_SLOT_ATTR and SLOT_FALLBACK_FRAG all sit behind that idiom and all stay. Verified on the full unit suite and on the browser suite across Chromium, Firefox and WebKit, which is where the router-client and slot halves of this actually run. * refactor(core): move three router-client primitives off the orchestrator navigator.js was a hub in both directions, and three of the things reaching back into it were not orchestration at all. buildHaveHeader is four lines over collectBoundaries, and four modules imported it from navigator.js. It moves to boundaries.js, beside the function it calls, which drops four edges into the orchestrator at once. `enabled` is one bit that events.js and prefetch.js both gate on. It moves to state.js, which already owns the router's module state, with navigator.js keeping the transitions through _setEnabled. Parking a shared bit beside the code that flips it is what pulled two leaf-ward modules into the cycle. diagnostics.js imported `navigate` and never called it. The dead-import sweep missed this one because the identifier does appear in code, inside the string literal `navType !== 'navigate'`, so a word-boundary match reads it as a use. Cyclic modules in this directory go from 11 to 8. The remaining eight are the router's genuine mutual recursion, which is a separate problem from misfiled code. * refactor(core): move the anchor lookups to a router-client leaf closestAnchor and findAnchorInPath are pure DOM walks over their own argument, but they lived in events.js, so prefetch.js and upgrade.js imported the router's event layer just to resolve an anchor. Moving them to anchors.js drops prefetch.js out of the cycle and takes the cyclic component from eight modules to seven. * refactor(server): split the check rule engine out of one function checkConventions held all twenty rules inline in a single 900-line function, so check/runner.js was 1298 lines and the issue's own complaint, that the rule engine had been relocated rather than split, was accurate. Each rule was already a self-delimited `// --- Rule: x ---` block that reads `files` and pushes to `violations`, so each becomes a named function, grouped by what it governs: components, routing, typescript, actions, registry, imports. The blocks move verbatim, keeping their comment headers, their logic and their order, and checkConventions becomes a twenty-line driver that reads as the rule list it always was. The support functions move to runner-support.js. Left where they were, every rules-*.js would import runner.js while runner.js imported them back, which is the cycle this PR is trying to remove rather than add. runner.js goes from 1298 lines to 111, and no module in the directory now exceeds 369. One rule was a comment plus a single call to one of those support functions, so the wrapper is gone and its explanation now sits on the implementation. webjs check still passes on gallery, examples/blog and website. * refactor(server): split the request serving out of dev/handler.js handler.js was 1460 lines holding two jobs: building and configuring the request handler, and serving whatever a request turns out to be. The second half moves to serve.js: the framework's own static files, an app source module with TypeScript stripped and elision applied, and the per-segment middleware chain. The seam is real rather than arithmetic. Nothing in serve.js builds or configures a handler, and handler.js reaches into it at exactly three points. The dependency is one way, so the directory stays acyclic. `exists` moves to helpers.js, since serve.js calls it eleven times and handler.js twice, so it belongs to neither exclusively. handler.js 1460 to 765, serve.js 711, and the dev barrel still exports the same sixteen names. * test(architecture): enforce the amended D3 and D4 mechanically The size ceiling and the cycle budget are only useful if they hold after this PR, and both were being checked by hand. module-size.test.mjs asserts nothing in the ten split trees exceeds 1000 lines, with the two exemptions named, capped, and carrying the reason each was granted. It also fails if an exempt file drops under the ceiling, so a stale exemption gets deleted rather than accumulating. import-cycles.test.mjs asserts the cyclic set is EXACTLY the two documented components. A new cycle fails it, and so does one of the two disappearing, which keeps the record honest in both directions. Writing the exemptions into a PR body would have left the next person to re-derive them. This way the reasons sit next to the numbers they justify. * fix(core): restore the anchor lookup events.js still calls Moving findAnchorInPath to anchors.js took it out of events.js without importing it back, so onClick threw a ReferenceError on the first click and the viewport prefetch observer then blew up behind it. Node's suite could not see this: the failing path is a real click in a real browser. I ran only the node tests after that move, which is the gap. The browser suite reds on all three engines with it, and is green with it fixed. * fix(server): import reachableFromEntries into the dev handler The merge that ported main's #1401 into the split moved the gate expansion onto reachableFromEntries, but the import never made it into the merge commit: it was staged before the fix and committed after, so the fix sat unstaged and only the working tree had it. Every suite I ran against that working tree was therefore green while HEAD threw a ReferenceError out of ensureReady on the first request. * fix(core): restore the slot rescue the render-client split dropped clearInstance had `if (p.kind === 'slot') { }`, an empty block where main detaches the record-owned children before the teardown disposes the slot subtree. That is the #1015 guarantee that projected children are values: the record keeps the refs, so a re-created slot re-places the SAME nodes. Without it a container-level template swap tears them down. rescueAssignedNodes was still exported and had no caller anywhere on the branch; this was its only one on main. Also folds the second updateInstance back into the one in parts.js. The two bodies were near-identical and had to be edited in lockstep, in a PR whose point is DRY, and the reconciler's copy minted a fresh Symbol('webjs.commitFailed') per throw instead of using the module sentinel whose comment explains why it exists. The hardcoded `const MARKER = 'wjm-'` goes back to the MARKER in html.js that parts.js already imports. * fix(server): undo four behaviour changes the ssr split introduced Each of these is a rewrite the split made while moving code, and none is mentioned anywhere. getNonce fell back to a client-supplied `x-webjs-csp-nonce` header. That header name appears nowhere else in the repo on either branch; it is invented. The JSDoc immediately above it still said the value comes from the request-scoped store and the argument is ignored, so the doc contradicted the code. cspNonce() wins when CSP is on and escapeAttr prevents breakout, so it is not directly exploitable, but a request header feeding the nonce on the boot script is not something a split should introduce. The 404 and 500 responses started passing the page's merged metadata to htmlResponse, which sets `cache-control` from `metadata.cacheControl` and has no non-200 guard. Its comment, carried over verbatim, justifies the missing guard with "every caller of THIS builder passes no metadata", which the change made false. An app setting cacheControl on a root layout, the documented pattern for a visitor-identical app, would serve its notFound() 404s publicly cacheable at the page's own URL. ssrBoundaryHtml was rewritten rather than moved: it emitted err.stack with no dev gate, so a throwing not-found / forbidden / unauthorized module put a server stack trace on the page in production, and it passed the raw heading as the title, turning `Forbidden` into `403: Forbidden`. escapeAttr and escapeHtml gained `>` escaping and `?? ''` coercion. Both decide served bytes, so both change every affected ETag. Restores the ten functions whose code was byte-identical to main's modulo comments, and splits the response layer into responses.js: the restored comments took render.js back over the 1000-line ceiling, which is the size guard doing its job. * fix(server): repair the dev app-source signal and drop a stray watch rule fileByteHash was rewritten as async while its only call site still interpolates it directly, so every entry in the app-source id became `[object Promise]` and the id stopped changing when app source changed. That kills the #899 deploy signal the client uses to evict stale caches. The branch is `if (!dev && state.moduleGraph)`, so it is production-only and no test could see it. Restored to main's synchronous 16-char form. frameworkServerVersion lost its `replace(/[^\w.-]/g, '').slice(0, 32)` sanitizer, and its failure fallback changed from '' to '0.0.0', which makes a failed read indistinguishable from a real version. The value is concatenated into the same id. dev/config.js also carried a second shouldIgnoreWatchPath with a different signature and a different rule set, missing the db/dev.db and db/migrations carve-outs, sharing a name with the live one in dev/server.js. Nothing imported it and main has no counterpart. * fix(server): de-duplicate the vendor helpers and drop the dead scanner fetchIntegrity existed twice. pins.js kept main's version with its `hash <url> returned <status>` / `failed: <why>` diagnostics; audit.js had a copy that returns null silently, and audit.js's copy is the one updatePinned calls, so `webjs vendor update` failed to hash a bundle with no message at all. One implementation now lives in integrity.js beside sha384Integrity, along with the PIN_BUNDLE_TIMEOUT_MS that was also declared twice. scanner.js still carried the pre-#1401 filesystem scanner: IMPORT_RE, DYNAMIC_IMPORT_RE, stripComments, isServerOnlyFile, CONFIG_FILE_RE and walk, which main deleted in the very commit this branch merged in. walk was only ever called by itself. The doc comment right below it says the function "no longer has a scanner of its own", which was true of the code that runs and false of the code in the file. Its ModuleGraph type path was also left at ./module-graph.js, one directory too shallow. resolvePackageDir gained a fallback to createRequire(import.meta.url), so a package the app never installed but the FRAMEWORK has resolves and gets vendored. In this monorepo, where everything hoists to the root, that is most of them. * test(architecture): catch a lost import, and fix two floors that could not fail Four defects on this branch were the same shape: a split moved code and left a call behind without its import. Each surfaced late and by luck, because none of them throws where a test looks. findAnchorInPath only runs on a real click, exists sat inside its own try/catch so it silently returned false, and reachableFromEntries and renderToString were behind a warm-up and inside a ReadableStream respectively, so both read as a wrong result rather than an error. no-free-identifiers.test.mjs reads every bare `foo(` call in the ten split trees and asserts the name is declared or imported. It is deliberately narrow and errs toward silence: anything that could be a local, a param, a property or a global is skipped, and a name is only reported when it appears nowhere in the file in a binding position, which is exactly what a lost import looks like. Proven by removing rescueAssignedNodes' import and watching it fail. The barrel floors for vendor and ssr were each one below the real export count, so either barrel could lose an export without failing. Every other floor equals its count. Also reconciles module-structure.md, which this branch added: it stated that no CI guard enforces the size ceiling and that nothing imports upward, both of which the guards added here contradict. The doc now says what is actually true, that the size gate is scoped to these ten trees and that two subsystems are genuinely mutually recursive and named. * fix(server): one escaper pair for SSR, and cover the split regressions The previous commit reverted the widened escaping in the copy it had just moved to responses.js and stopped there. main had ONE pair serving every call site; the split made three, and the two in head.js and env-shim.js still escaped `>` and coerced with `?? ''`. head.js serves `<title>`, every `<meta content>`, every `<link href>` and `integrity=`, so most of the divergence was still live: `a > b` in a title served as `a > b`. All three now import one pair from ssr/escape.js. Adds the tests these fixes should have shipped with. Every defect this round found survived a fully green suite, so "the suite passes" was not evidence of anything. Each test is proven against the defect it names by reintroducing it: the 404 cache-control inheritance, the request-header nonce, the production stack trace out of a throwing boundary, the widened escapers (asserted on served bytes, so a third copy reappearing in head.js fails it), and the app-source id, which is observable through `x-webjs-src` in prod and whose frozen-id shape needs the change-detection assertion to catch. Also closes a blind spot in the free-identifier guard: it accepted `,NAME` and `NAME,` anywhere in the file as a binding, which a call ARGUMENT satisfies, so a lost `publishedBuildId` import called as `headers.set('x-webjs-build', publishedBuildId())` passed. That is the exact defect class the guard exists for. It now catches it, and it caught a real one on the way in: responses.js still using escapeAttr after I removed its import. Remaining cleanups this round turned up: the ModuleGraph JSDoc path the last commit claimed to fix and did not, a `templateCache` import left dead by the updateInstance de-duplication, the PIN_BUNDLE_TIMEOUT_MS rationale left behind in BOTH files that no longer declare it while its new home got a one-liner, a dangling `@param` where the escapers moved out, and a responses.js header claiming four builders when privateFragment stayed in render.js. * fix(server): restore the enforcement gates the split silently disabled The split moved the real code out of `src/ssr.js` and `src/dev.js` into `src/ssr/*` and `src/dev/*`, and the hooks that gate this repo match on path. `require-bun-parity-with-runtime-src.sh` and the reminder in `require-tests-with-src.sh` both keyed on the literal `/ssr\.js`, `/dev\.js`, `component\.js` and `slot\.js`, none of which match a nested file, so every runtime-sensitive edit this PR made sailed past a gate that would have blocked the same edit on main. That is why 23 findings across two review rounds all landed on a green suite. packages/server/AGENTS.md already documents this exact trap and says to widen the pattern in the same PR that creates the directory. The PR wrote the instruction and did not follow it. Widened both patterns to `/ssr[./]`, `/dev[./]`, `component[./]` and `slot[./]`, with tests proving the gate now fires for the five split ssr and dev modules and still does NOT fire for `component-scanner.js` or `component-elision.js`, which sit beside the runtime path and are not on it. Proven by reverting the pattern and watching the new case go red. Adds the cross-runtime assertion the restored gate asks for (`test/bun/ssr-escape-parity.mjs`): the escapers, the 404 cache-control and the boundary error path all serve identical bytes on Node 26.7.0 and Bun 1.3.14. Drops one assertion from the last commit that could not fail: the app-source id is a sha256 of its input, so the header is hex whatever goes in and `!src.includes('object Promise')` was unfalsifiable. The sibling change-detection test is the one that actually catches a frozen id, and it does. Syncs packages/server/AGENTS.md, whose ssr.js row listed three of the eight sub-modules, and corrects a comment in importmap.js that named ssr.js as the source of truth for attribute escaping when ssr.js is now a barrel that defines none. * docs(core,server): restore the comments the splits dropped Five of the ten splits rewrote function bodies while moving them, and between them dropped roughly 1,800 explanatory comment lines. The other five dropped zero, which is what a faithful move looks like and is why this is a defect rather than a fact of splitting. The PR's own audit counted only `/** */` blocks, so it read clean. This restores 1,331 of them by re-attaching each comment block to the code line it sat above: main's body is split into code lines and the blocks between them, the code lines are aligned against the current body, and each block is inserted above the line its anchor aligned to. Only comment lines are ever inserted, and the tool re-strips the result and refuses the file unless the code lines are byte-identical to what was already there. That constraint is the point, because these are the exact five trees whose rewrites produced every defect the review found: this cannot revert one of those changes, and it cannot introduce a new one. 195 lines are still unaccounted for. They are module-scope comments between top-level declarations rather than inside a function, so they have no anchor this pass can use, plus a handful whose anchor line no longer exists at all. The size guard then failed, correctly, and that turned out to be the more interesting result: `parts.js` and `dev/handler.js` went back over a ceiling they had only been under BECAUSE the documentation was missing. A gate that reads restored explanation as a regression is measuring the wrong thing, and its cheapest remedy is deleting comments, which is the defect being fixed. So the guard now counts CODE lines. Measured that way every module in the ten trees is under 1000, including both former exemptions (`parts.js` 938 code lines inside 1986 raw, `lifecycle.js` 535 inside 1481), so the exemption list is now empty, which is where the plan wanted to land and where a raw count could not. * docs(core,server): restore the dropped comments as whole blocks Redo of the previous restoration, which filtered a block LINE BY LINE against what was already present. When a block's opening lines happened to exist elsewhere, only the remainder landed, and what it left behind was a sentence starting mid-clause under unrelated code. The twelve-line comment on the lazy-analysis stages ended up as its eleventh line alone, sitting above `let analysisDone = false;`. Both passes are atomic now: a block is either already present in full, or it goes in in full. That restores fewer lines than the fragmenting version (1,306 against 1,400) and every one of them is a whole thought. 1,306 of the 1,525 restored. The remaining 219 are blocks whose anchor line no longer exists in the split, mostly inside the two functions the split restructured hardest rather than moved, so there is no honest place to put them mechanically. * docs(core,server): place the last mechanically-placeable comments Two more passes over what the function-aligned restore could not reach. The first ignores function boundaries entirely and matches a block's anchor line, compared on CODE only, across every file of the tree, placing it only where that line occurs exactly once. That is what the earlier pass could not do for `createRequestHandler` and `wrapHead`, whose bodies were restructured far enough that difflib stopped aligning them. It accounts for 113 lines. The second is a hand-built map for blocks whose anchor is ambiguous or gone, naming the target line for each. The TEXT is still copied out of origin/main rather than retyped, because hand-typing is how a paraphrase gets in: one did during this pass, in the `clearVendorCache` note, where two lines came out as my words instead of main's and had to be corrected against the original. A drifted comment is the defect being repaired here, so the tool does the copying. 1,448 of the 1,525 now restored. The remaining 77 are blocks whose anchor genuinely no longer exists, and placing those means deciding what they now describe rather than where they go. * docs(server): place the last four sited comment blocks The #254 redirect ordering, the #255 trailing-slash rule, the framework-static early path and the CSP header note, each mapped to the line it documents and copied verbatim from origin/main. 1,474 of the 1,525 restored. The remaining 51 are blocks whose anchor code no longer exists in any recognisable form, so placing them means deciding what they describe now rather than where they go, which is authoring rather than restoring. Four of them are one-liners over re-export statements the split rewrote (`// Re-export for unit testing.`), and the rest sit in `wrapHead`'s metadata walk and the listener context, both restructured rather than moved. * fix: repair the damage the comment restoration did The restore de-duplicated by comparing LINE text. The split had re-wrapped several paragraphs, so the same prose at a different line width matched nothing and went in a second time, and because the two copies land adjacent they form ONE contiguous comment block, which a block-level check does not see either. Three paragraphs ended up duplicated: the #756 trusted-IP note in dev/handler.js and two in ssr/head.js. Two of those duplicates were worse than noise. The `_metadataIconRoutes` copy re-introduced main's "the same shape as setClientRouterEnabled ABOVE", which is false now (that function lives in ssr/client-router-flag.js), over the top of the corrected wording the split had written. The client-router-flag copy did the same thing to a module JSDoc that already said it accurately, and re-asserted that `dev.js` reads the config when it is dev/handler.js that does. Two blocks landed somewhere they are not true. `// Swallow rejection. A rejected Promise is treated as "no value"` was the body of the REJECTION handler in main; the split collapsed that to `() => {}`, so the restore put it at the end of the FULFILLMENT handler, describing the success path it names as the failure one. And a bare `// ignore` landed at column 0 after a whole try/catch, documenting nothing; it is the catch body now. Ten more lines were re-indented to the depth of the code they document. Also two problems in the gate widening from 34dd5b6d. Its test asserted the component/slot patterns against require-bun-parity, whose regex has never contained either word, so it proved nothing about the hook that actually changed; there are now tests that drive the client-facing reminder itself, proven by reverting the pattern. And `component[./]` matched `component.d.ts`, a file with no runtime, so the pattern is anchored. Finally, the figures this argument rests on are now asserted rather than quoted: the header claimed parts.js was "938 code lines inside 1986 raw" and was wrong three commits later. The test checks the relationship those two files have to hold instead. * fix: restore two comment indents the re-indent pass misattributed The re-indentation took each block's indent from the next code line below it, but did not treat `} catch (...)` / `} else if (...)` as continuations of an enclosing construct. Both are code lines starting with `}`, so the pass read their indent as the block's and de-indented two comments out of the branch they document: `component/lifecycle.js` moved the `shouldUpdate=false` note from inside the `try` body, where it explains the branch that just closed, to align with `} catch (preCommitError)`, where it reads as documenting the catch. `ssr/render.js` did the same to the `absolute` note, moving it out of the `if (typeof t.absolute === 'string')` branch it describes and onto the `else if`. Both are back at main's indent, byte-identical to it. Swept the ten trees for the same shape (a comment block whose next code line is a brace continuation, indented at or below it) and there are no others. * fix: correct the JSDoc type paths the split left one level too shallow Moving a module a directory deeper breaks its relative type references as surely as its runtime imports, but only the runtime ones fail loudly. A JSDoc `import('./x.js')` that no longer resolves degrades the annotated symbol to an unresolved type in silence, and `packages/` has no tsconfig, so nothing in CI looks. 21 of them across seven modules, including the whole `ReloadVerdict` contract #1405 threads through `onReload`, `rebuild`, `doRebuild`, `classifyWatchPath` and `pendingVerdict`, whose runtime import the merge corrected while leaving the five type references pointing at `dev/dev-classify.js`. They were surfacing one review round at a time, so this adds the check that finds them as a class. It skips the three specifiers that appear in prose as illustrations of what an app author would write, matched exactly so a real reference cannot hide behind one. Proven by pointing one back at the wrong path and watching it fail. * test(architecture): scope the prose-example exemptions to their file The type-path guard excused three specifiers by name, which excused them everywhere in the ten trees. They are illustrations of what an APP author would write, and each belongs to exactly one file, so a genuinely broken `import('./x.ts')` in any other module would have been waved through by an exemption earned somewhere else. Keyed by file now. Proven by adding that specifier to slot/project.js and watching it fail while serve.js, which legitimately has it in prose, still passes. * fix(server): restore the head order and five other main divergences The head one matters most and I had claimed the opposite. An earlier comparison of rendered bytes between main and this branch reported "identical apart from a clock"; that comparison ran against a HYBRID tree, because `git checkout origin/main -- packages/` restores tracked files without deleting the branch-only ones, so it proved nothing. Redone against a clean worktree of main, the served `<head>` differs: main emits seven modulepreload hints, `@webjsdev/core` among them, BEFORE the icon / apple-touch-icon / canonical links, and this branch emitted them after, because the split moved the preload block to the end of `wrapHead`. That is a boot-critical hint-discovery change on every page. The block is back where main has it, verified by re-rendering and diffing the tag order. Five more, each a rewrite the split made while moving code: `<link rel="author">` was pushed to `linkTags` rather than `metaTags`, which the document template joins before `<title>`, so it moved in the head for any app declaring `metadata.authors[].url`. `cachedHtmlResponse` grew fallbacks main does not have, and one of them, `rec.body || rec`, puts the RECORD object in the response body for a cached record with an empty-string body, contradicting its own `@param`. There is one call site and it always passes a well-formed record, so the fallbacks bought nothing. The base-path-miss 404 lost its `content-type: text/plain`. The framework probes were matched against the DECODED path, so `/__webjs%2Fhealth` answered the liveness probe; they match the raw pathname again, guarded, as main does. `/__webjs/reload.js` and `/__webjs/reload-worker.js` were gated on `dev &&`, so in production they fell through the whole pipeline instead of returning the explicit 404 main returns, and they had gained a `cache-control` header main does not send. Also raises the router-client barrel floor to 69, which the #1405 merge left at 68 by adding `refreshPage` without it, and adds a test that every floor EQUALS its export count. A floor below the count tolerates losing exactly that many exports, which is the regression the guard exists to catch. * fix(server): restore three guarantees the split quietly dropped All three came out of the review round on this PR. None was reachable today, and each is the same failure shape the split has already produced once: a guarantee that survives as a comment after the code behind it moved or was copied. The pin directory had two owners. `vendor/pins.js` WRITES the pinned bundles from `PIN_DIR_REL` while `vendor/resolver.js` READ them from its own hardcoded copy of the same path. They agreed, so nothing failed. A change to `PIN_DIR_REL` would have moved the write without the read, and the resolver would then have missed every pinned bundle and fallen back to a live vendor resolve with no error. That is the 247-line `wrapHead` duplicate again, so the fix is one owner rather than two copies. `publicEnvShim` and `wrapHead` had grown fallbacks main does not have. `opts?.env`, `opts?.dev`, `opts?.nonce` and `opts.moduleUrls || []` turn a missing required argument into a silent wrong answer: a production env shim, or an importmap with no imports and no modulepreloads, where main threw. `publicEnvShim` is a public export of `ssr.js`, so this was an observable change to its contract. The `|| []` was not even applied consistently, which is how it reads as incidental rather than intended. The two dev reload assets were reimplemented inline in `dev/handler.js` and dropped from `tryServeFrameworkStatic`. The helper's second caller, the `handleCore` fallback, exists precisely to keep those assets serving if a future caller ever bypasses the early path, and its comment still promised that. They move back into the helper, which is the same one-implementation rule #1397 applied to `tryServePublicAsset`. Verified byte-identical to main: the 15-route SSR corpus, and the reload endpoints in dev and prod. Both new tests fail when the fix is reverted. * test(architecture): measure D3 with raw lines and named exemptions The size guard counted CODE lines (comments and blanks skipped), which put every split module under the 1000 ceiling with an empty exemption list. That redefinition is reverted: #1365 specifies the raw `wc -l` count plus a NAMED exemption for a module that genuinely cannot be split, and changing the metric so a failing criterion passes is not meeting it. The guard now reports the number you see when you open the file. Three exemptions are named, each with a cap and its reason: - component/lifecycle.js (1481, cap 1600): lit parity. The file tracks lit's reactive-element.ts, which lit keeps whole at 1754 lines, and the standing decision is to keep lit-derived code close to lit. - render-client/parts.js (1991, cap 2100): mutual recursion. The apply and instance group calls back into itself, so a real split creates the cycle D4 forbids; lit keeps its equivalent whole at 2303. - dev/handler.js (1386, cap 1500): one closure over shared request state; decomposing it rewrites every app's boot path for zero behaviour gain. An exemption whose module shrinks under the ceiling fails the guard, so the list cannot hold stale entries. module-structure.md is aligned so future agents inherit the decided rule, including the reason the comment-density tension is answered by the exemption list rather than by a different metric. * docs(core,server): restore the last comment blocks the splits dropped Closes the ~58-line documentation gap the PR body carried as the only outstanding item, so #1365 needs no follow-up. Re-ran the comment-line multiset comparison between each pre-split monolith on main and the tree it became. Of the lines it reported as missing, these were genuine losses and are restored at their anchors: - the repeat reconciler's note on why the push sits BEFORE the removal (a pure reordering that keeps a built-and-inserted slot tracked at every throw point) - the applyChild fallback's note on why the generic path is safe when no cached instance is available - the `until` directive's two priority-slot notes (why a sync candidate beats a rendered Promise, and the all-Promise first render) - the SSR prop-attr parser's note on why a malformed payload is skipped silently (undefined-prop semantics, hydration fails the same way) - the streaming renderer's `ssr: false` note - the CSP catch, which the split had reduced to `/* ignore */`, losing the reason (a malformed policy must fail closed to no header rather than 500 every request) - the listener context's note on what the two shells share and why - the modulepreload emitter's #256 + #243 note on why `crossorigin` and `integrity` are decided on the ORIGINAL url - the `?v=` fingerprint note at the early static path - the nine undocumented `_`-prefixed test re-exports in ssr/ The remainder of the reported lines are not losses, and are left alone: section banners internal to a monolith (the module is now the section), JSDoc re-pathed one level deeper by the move, paragraphs the split re-wrapped at a different width (the #756 security block is present and intact), blocks the split's own wording supersedes (the client-router flag, the metadata icon routes), and the docs for `teardownUntil`, which was write-only dead code on main and correctly removed. Comment-only, verified per file against HEAD. The one apparent code delta is `catch {}` reflowed to hold its restored comment, which is the shape main has. SSR stays byte-identical to main across the 15-route corpus. * test(architecture): drop the LOC guard D3 forbids D3 rejects a line-count CI gate twice, in its own reasoning ("Do not add one") and again in Out of scope ("No LOC CI guard. Reasoned and rejected in D3"), on the grounds that it is a proxy metric fighting cohesion and that it must carry an exemption list that rots. The criterion it specifies instead is a one-time acceptance check, with any exemption argued in the merging PR. This branch added the gate anyway, first counting code lines so it passed with no exemptions, then counting raw lines with three. Both were me substituting a mechanism for the one the issue chose, which is the same error the code-lines metric already was. So the guard goes and the three exemptions move to the PR body with their measured sizes and reasons, which is the form D3 asks for. The module-structure reference is aligned: the ceiling is a review-time check with the command to run, not a test, and it now says explicitly that "it is mostly comments" is not a valid exemption reason (say why the CODE cannot be split, or split it). The other four architecture tests stay. They guard export surface, cycles, free identifiers and type-import paths, none of which is a proxy for anything. * docs: point the framework-source references at the split trees #1365's Docs table assigns a specific edit to each doc surface that names a framework source path, because those sections exist to tell a cold agent where to look and the split changed where to look. Several were still pointing at a barrel as though it held the code, which is the #488 staleness the doc gate was written for: an agent following "the SSE push in packages/server/src/dev.js" opens a 23-line re-export file and finds nothing. - AGENTS.md "Framework source": the four starting points still resolve, since the barrel keeps the path, so the fix is a sentence saying each IS a barrel and the code is one level down, with the other six named. - framework-dev.md: the vendor fetch-callers-all-catch claim now names the four modules in `vendor/` that actually fetch; the core-publishes -first claim points at `dev/handler.js`; the dev-overlay mechanism points at `dev/handler.js` and the `ssr/` tree. - packages/core/AGENTS.md: the metadata surface points at `ssr/head.js`, which is what reads and constructs it. - components.md: the base-surface grep advice adds the sibling `component/` directory, where the class body lives. - packages/mcp: four example strings used `server/src/ssr.js` as the illustration of a readable source path. The `source` tool reads any path under the src trees, so nothing was broken, but an agent copying the example landed on a barrel. They now show a real module and say the bare path is a barrel. Docs only, no source touched. Suite unchanged.
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
1 participant
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.
Closes #1398
Summary
Every dev edit produced a full page reload: the page blinked, hydrated component state reset, scroll position was lost. Most dev edits do not need that. Pages and layouts never hydrate, so a freshly rendered page is the complete truth for them, and the client router can swap it in place instead.
The dev watcher now classifies the changed file against the module graph and puts a ternary verdict on the SSE
reloadframe. The browser picks the lightest correct response:pageshellreloadlocation.reload(), exactly what it always didComponent edits stay a full reload by design.
customElements.defineis once-per-tag, so a morph would apply new markup wired to the old class. Nothing here hot-swaps a module, so the deferred HMR entry inAGENTS.mdstays, clarified rather than removed.What changed
packages/server/src/dev-classify.js(new). A pure, state-free ladder: cold analysis reloads, outsideappDirisshell, in the shipped closure reloads, a page morphs, anything else in the graph isshell, and everything else reloads.strongerVerdictorders the three so a batch collapses to the strongest.dev.js.ensureReadyderives the shipped closure from the entries elision KEEPS (deliberately notbrowserBoundFiles, which is the authorization gate and walks elided entries too). The watcher holds the strongest verdict across its 80ms debounce and threads it toonReload.listener-core.js.SseHub.reload(verdict)emits a single-line JSON payload. An absent or malformed verdict is emitted asreload.dev-reload-worker.js.parseVerdictresolves anything unreadable toreload, and the relay takes the strongest verdict per coalesced batch. A changed boot id contributesreloadunconditionally.router-client.js. NewrefreshPage(mode), the full-body tier hoisted intoswapFullBodyso it has a second call site, and arefreshflag throughperformNavigation/fetchAndApply/applySwapthat suppresses the outgoing snapshot, the loading skeleton, history, scroll, the prefetch fast path, andX-Webjs-Have.Suppressing the have-header is required, not an optimisation: the server short-circuits at the first layout the client already holds, and a same-url request matches every one of them, so the response would omit the very layout that changed.
Reach, stated honestly
The morph needs the server process to survive the edit, so it applies on Bun's
bun --hot, underwebjs dev --no-hot, and for Node edits outside the five--watch-pathdirs. Anode --watchrestart carries no filename, so nothing survives to classify and it stays a full reload. That is written into the runtime docs rather than glossed.Test plan
packages/server/test/dev/classify-watch-path.test.js(the whole ladder plusstrongerVerdictover all nine ordered pairs), extendedlistener/listener-core.test.js(the frame shape and its fail-safe), extendeddev/reload-shared-connection.test.js(the served client's feature detection and single readiness loop).packages/server/test/dev/classify-live.test.jsboots a real dev server and asserts the frame end to end. This is the counterfactual for dropping the filename in the watcher again, which would make a page edit and a component edit byte-identical on the wire.packages/core/test/routing/browser/refresh-page.test.js(what survives a swap, against real DOM identity and real scroll) and a new suite inpackages/server/test/dev/browser/reload-worker.test.js. The strongest-verdict rule gets an order-independent counterfactual that a last-write-wins relay fails.test/e2e/dev-morph.test.mjs, three cases in a real browser against a realwebjs dev. Wired into CI.test/bun/dev-morph-verdict.mjsplus its wrapper. The frame ridesSseHub._raw, which the two listener shells drive over different transports, so the JSON payload is proven identical on both. Wired into the Bun CI step.Docs
AGENTS.md(public API table, thewebjs devCLI line, the clarified deferred-HMR note),packages/server/AGENTS.md(adev-classify.jsmodule-map row, the classification prose, theSseHub.reload(verdict)row),packages/core/AGENTS.md(therouter-client.jsrow), the skill'sreferences/runtime.md(a new row in the Node-versus-Bun table plus the honest reach) andreferences/client-router-and-streaming.md, and the docs site atwebsite/app/docs/runtimeandwebsite/app/docs/client-router. The scaffold copy of the skill is synced from the repo-root canonical at prepack, so there is no separate mirror to edit.MCP: N/A, it serves the docs corpus and
AGENTS.md, both updated, and no introspection projection changed. Editor plugins: N/A, no grammar, snippet, or language-service change.README.md: N/A, this is not a headline capability.CONVENTIONS.md: N/A, no new convention. Scaffold generators: N/A, generated code is unchanged.