docs(skill): teach component-ownership principles, not just the API by vivek7405 · Pull Request #1328 · webjsdev/webjs

vivek7405 · 2026-08-08T12:28:46Z

Summary

The skill taught the WebJs API well but never said how to choose between two ways of doing the same thing. An agent could learn that createRef() exists from one row in the directives table without learning that a ref is the preferred way to reach a rendered node, and nothing in the skill mentioned document.querySelector as an anti-pattern at all. That gap produced code that passes webjs check, passes tsc, and is still wrong in the way that costs the most later. This repo's own website was the worked example (#1222, fixed by #1223): two UI surfaces driven by delegated document listeners in the root layout, coupled to their markup by a class selector, with open state parked on <body> where it outlived every client-router swap.

This adds a ## Ownership: what a component owns section as the first content section of references/components.md, stating seven rules with the concrete failure each prevents, then routes to it from the four pointer surfaces and mirrors it on the docs site.

What changed

.agents/skills/webjs/references/components.md. The new section plus a matching first bullet in ## What This Covers. It lives here rather than in muscle-memory-gotchas.md, which is framed by ORIGIN ("coming from Next.js" / "coming from Lit"): only the delegated-listener habit has an origin, while the rest are consequences of the WebJs execution model, so they would sit under a heading that misdescribes them.
.agents/skills/webjs/references/muscle-memory-gotchas.md. The vanilla-DOM entry now distinguishes the legitimate document LISTENER from the document QUERY and points at the new section instead of restating it.
.agents/skills/webjs/SKILL.md. The components routing cell, plus two Common Mistakes To Avoid bullets.
AGENTS.md. The detail-docs routing row only. The rules stay in the skill, which is where AGENTS.md routes for this depth.
website/app/docs/components/page.ts. The public equivalent, so the docs site does not go stale the way feat: HTTP-verb server actions via config exports (GET/POST/PUT/PATCH/DELETE) [epic] #488 did.
Two gallery comment annotations, so the reference a scaffolded app ships explains WHY it uses a ref and WHY its one document listener is the legitimate case.

Decisions

Rule 6 permits the document listener rather than banning it, with the decidable test being what the handler READS: this.contains(e.target) is a decision about the component's own subtree, document.querySelector('.other-thing') is a decision about someone else's markup. A blanket ban would make the scaffold's own gallery-nav.ts listener a violation.

Rule 7 exists because the skill had zero coverage of CustomEvent / dispatchEvent, so cross-component communication had no stated answer and an agent's fallback was the document bus this change discourages.

No webjs check rule is added, deliberately. Root AGENTS.md draws the line at "could a sensible app legitimately want this to pass", and a sensible app can want a delegated listener to pass. No new reference file either, since the judgement belongs where an agent is already reading when it authors a component.

The skill is committed ONCE. packages/cli/templates/.agents/skills/webjs/ is gitignored and generated at prepack, so editing the canonical copy IS the scaffold's update; the generated path was verified absent and unstaged.

Test plan

node --test test/docs/doc-source-consistency.test.mjs test/docs/docs-pages-well-formed.test.js test/docs/llms.test.mjs test/knowledge/knowledge-coverage.test.js test/scaffolds/scaffold-template-validation.test.js passes, 34/34. The first of those is the guard that can actually red this change, since it resolves every named framework import in the skill and website corpus against the live export universe.
node --test test/scaffolds/scaffold-integration.test.js test/scaffolds/scaffold-gallery.test.js passes, 23/23.
Generated a fresh app with webjs create, confirmed it carries the new skill section and both gallery annotations, and webjs check reports all checks pass.
Reviewed over seven rounds. Every round through the sixth found a real factual error in the new prose (a rule contradicting another file in the same skill, a carve-out that condemned code the scaffold ships, a claim naming the wrong router mechanism, a signal-lifetime claim false for a module-scope signal); the seventh, a fresh whole-diff pass, came back clean with every claim verified against the framework source.
Dogfood: website boots in prod mode and serves /, /docs/components, /ui and /ui/button at 200 (and /docs at 308) with no broken modulepreload hints. Blog e2e N/A, since nothing under packages/*/src changed. Note for anyone repeating this from a fresh worktree: website/modules/ui/components/ is gitignored and mirrored from packages/ui, so /ui/* 500s until you run node website/scripts/copy-registry.mjs. That is a worktree setup artifact, not a defect in the branch.
No counterfactual, stated honestly: reverting a prose section reds nothing and reverting a code comment reds nothing. The equivalent gate is the invariant-11 hook on every added line plus the four content guards above.

Doc surfaces

Every surface is in this PR, since the change IS a docs change. website/AGENTS.md needs no edit (it already carries the app-level version of these rules and is the precedent, not a surface to sync). README.md and the marketing pages are excluded: no public API surface and no headline capability. No packages/*/src change, so the Bun-parity and runtime-source gates do not fire.

vivek7405

Read the whole diff. The prose does the job I wanted from it, the seven rules each name a real failure rather than sitting at the level of generic advice, and rule 6 draws the listener-versus-query line in a way that does not accidentally condemn the gallery's own webjs:navigate listener. Two real problems though, both in the new section.

The one that matters is rule 5's claim about how often a layout's inline script runs. That is only true of the ROOT layout, and stated unqualified it contradicts client-router-and-streaming.md in the same skill and points an agent at exactly the bug #1102 exists to describe. Worth being careful here: this section is the first thing an agent reads before authoring a component, so a factual error in it propagates further than one buried in the API sections below.

The other is the canonical example binding a ref it never reads, which teaches the syntax without the payoff.

vivek7405

Went back over the fix commit and traced its claims out into the router source and the other doc surfaces. Two problems, and both are the same failure mode: the section is confident about the document shell in a way the framework itself is not.

Rule 3 and rule 5 disagree with each other seven lines apart. Rule 3 forbids state on the shell with a carve-out narrow enough to exclude the theme, and rule 5 then blesses reading the stored theme before first paint. The scaffold ships a toggle that writes data-theme on <html> and never releases it, and styling.md instructs exactly that, so the rule as written tells an agent to refuse code we generate.

The root-layout claim is the other one. I hardened a sentence that was already in the skill, and making it load-bearing is what exposed it: the snapshot-restore path does re-run body scripts. Worth noting the same wording lives in two other surfaces this PR does not touch, so correcting it here leaves them out of step. Deferring that rather than growing the diff.

vivek7405

This one is on my last fix, not on the original section. I softened the root-layout claim by blaming the in-place full-body swap, and that is not the path an ordinary back or forward takes. applySwap returns inside the keyed boundary path as soon as planBoundarySwap yields a plan, and a restore between two pages of the same app always yields one, so it never reaches the href-gated branches or the full-body swap below them. That path activates scripts only between the boundary comments, and the root layout's chrome sits outside its own children boundary, so its script is not re-activated. The sentence I replaced was closer to right than the replacement.

Restating it by the mechanism that actually decides it, the markup sitting outside every swap range, fixes the accuracy and also puts it back in step with client-router-and-streaming.md, which the same paragraph routes to. That resolves the desync without editing two files this PR does not otherwise touch.

vivek7405

Confirmed the last fix is right: the boundary path is what every ordinary navigation and restore takes, and the root layout's chrome sits outside it, so "outside every swap range" is the mechanism that actually decides it. Two problems left, both in the original section rather than in that fix.

The framing paragraph is the one that bothers me, because its entire job is telling a reader which of these rules are WebJs-specific and which are ported, and the partition it draws does not survive contact with the rules. It puts rule 3 in the imported group by citing React's lifting-state framing, then justifies "the other four" with the swap-range mechanism that is rule 3's whole basis, and leaves rules 6 and 7, which are ordinary component-model advice, on the WebJs-specific side. A reader trusting that split learns the wrong thing about what is actually WebJs-shaped here.

The SKILL.md bullet is the other one, and it matters more than its size suggests because SKILL.md is read BEFORE components.md is loaded on demand.

vivek7405

Three problems, and the shape of them is worth naming: each one is a small overreach in a sentence I wrote to FIX a previous overreach. Tightening prose keeps introducing a new claim slightly stronger than the mechanism behind it.

The hydration one is the real defect. "Cannot own behaviour at all" is not what the execution model says: a page or layout module loads in the browser and its top-level side effects run, which is exactly why the elision verdict needs a ships-whole case. What never hydrates is its MARKUP. Stated the strong way, the section denies the existence of a shape it is otherwise trying to warn about.

The other two are attribution: rule 5's WebJs-specific half rests on the swap range like rule 3 rather than on hydration, only the first of rule 4's two caveats comes from the renderer disagreement, and the docs site never got the second half of the carve-out that SKILL.md gained.

vivek7405

Three findings, and the pattern behind them is the point: two of the three are in the framing paragraph again, which is now the source of three of my last five findings. Each tightening of it swapped one slightly-too-strong claim for another. It was trying to sort the seven rules into ported advice versus WebJs-specific, and that partition does not survive contact with the rules, so the honest move is to stop drawing it. The prior art it cites is worth keeping and stays; each rule now names its own mechanism where it needs one.

The signal one is a real independent defect and the most valuable catch of the whole cycle. Rule 3 said state in a signal dies with the element, which is false for a module-scope signal, contradicts rule 7 four lines down, and contradicts the gallery file this PR annotates, where a module-scope signal exists precisely so the value outlives the element.

vivek7405

Read the whole thing again at head, this time checking every factual claim against the framework source rather than against the prose around it. It holds up: the ref semantics (SSR no-op, .value cleared on teardown), the synchronous webjs:before-cache snapshot read behind rule 4's direct-write exception, webjs:navigate being document-dispatched because the router has no element to dispatch from, the root-layout-versus-nested split against planBoundarySwap, the theme carve-out against the toggle the scaffold actually generates, and the claim that no webjs check rule covers any of this against the full rule list. The sample compiles against the documented API and cannot trip reactive-props-no-class-field, and the scaffold copy of the skill really is generated at prepack, so editing the canonical file is the whole sync.

Nothing left open. Clean.

Rule 5 claimed a layout's inline script never re-runs, which is true only of the ROOT layout. A page or nested layout sits inside the swap range and re-executes on every navigation that swaps it (#1102), so the rule as written contradicted client-router-and-streaming.md and taught the harmful direction. The ref sample also bound a handle it never read, so it showed the syntax without the reach or the after-first-commit timing.

Rule 3's carve-out covered only a transient effect released on disconnect, which excluded the theme the scaffold itself ships and rule 5 blesses seven lines later. It now names both shapes and points at the flag-on-the-shell case it actually forbids. Rule 5's root-layout sentence claimed a once-per-load guarantee. A popstate snapshot restore calls applySwap with href null, skipping both href-gated degradations and landing on the in-place full-body swap, which re-activates the scripts in body.

…cides it The previous wording blamed the in-place full-body swap, which a popstate restore only reaches when the boundary scan yields no plan. applySwap returns inside the keyed boundary path first, and that path activates scripts only between the boundary comments, which the root layout's chrome sits outside. Stating it as "outside every swap range" is both correct and back in step with client-router-and-streaming.md.

The framing paragraph claimed a clean three-versus-four split between imported advice and WebJs-specific rules, then attributed rule 3 to both sides and left rules 6 and 7, which are ordinary component-model advice, on the WebJs side. It now names the two execution-model facts that actually move the boundary and says which rules follow from them. The SKILL.md bullet banned state on the shell outright, contradicting the carve-out components.md, the docs site, and styling.md all state, and the theme toggle the scaffold ships.

…ve-out "Cannot own behaviour at all" overstated it: a page or layout module loads in the browser and its top-level side effects run, which is why the elision verdict has a ships-whole case. What never hydrates is its MARKUP. Rule 5's WebJs-specific half rests on the swap range like rule 3, not on hydration, and only the first of rule 4's two caveats comes from the renderer disagreement. The docs site kept the single-exception form after SKILL.md gained both, so the public surface still read as banning the scroll lock the site's own drawer uses.

The framing paragraph's attempt to sort the rules into ported versus WebJs-specific produced a new wrong claim on each tightening: three of the last five review findings landed in that one paragraph. The prior art it cites is worth keeping, the taxonomy is not, so each rule now names its own mechanism inline. Rule 3 also said a signal dies with the element, which is false for a module-scope one and contradicted rule 7, the directives section, and the gallery-nav file this PR annotates.

vivek7405 self-assigned this Aug 8, 2026