Skip to content

test(website): tag-balance guard only covers app/docs, not the rest of the site #1263

Description

@vivek7405

Problem

The container-tag balance guard covers app/docs and nothing else, so most of the site's hand-authored markup is unchecked.

test/docs/docs-pages-well-formed.test.js counts opens against closes for pre, code-block, div, ul, ol, table in every page's html template, and it exists because a real incident: an unclosed <pre> in website/app/docs/components/page.ts pulled the <!--/wj:children--> layout marker inside it, after which router-client.js reconcileSiblings threw NotFoundError from insertBefore on every client-router navigation after that page was visited.

Its glob (L97) is website/app/docs/**/page.{js,ts}. Everything else the site hand-authors is outside it:

  • website/app/page.ts (5 <pre>), website/app/why-webjs/page.ts (4), website/app/what-is-webjs/page.ts (1), website/app/error.ts (1)
  • website/app/ui/page.ts and website/app/ui/[name]/page.ts
  • the hub pages: app/articles/, app/blog/, app/compare/, app/changelog/, app/brand/, app/not-found.ts
  • website/app/layout.ts, which wraps every page, so unbalanced markup there breaks the whole site rather than one route

Nothing else in the repo checks tag balance; this is the only such guard. The failure it catches is silent at author time, survives review (the page renders fine on a hard load), and only appears as a client-router crash on a later navigation, which is precisely why it was worth automating for the docs.

This surfaced during #1249: that PR corrected a comment claiming pre stayed in the guard's list because other pages author it directly. They do, and the guard never sees them.

Implementation plan

Decision: the guard covers every website-owned module that authors page markup, in ONE renamed test file, and lands with zero violations because there are none. Three globs: website/app/**/{page,layout,error,not-found,global-error,global-not-found,loading,forbidden,unauthorized}.{js,ts} (62 files today, 60 with a template), website/components/**/*.{js,ts} (7), and website/lib/ui/**/*.{js,ts} (4). The shared-chrome globs are not optional decoration. app/articles/page.ts, app/blog/page.ts, and app/compare/page.ts author no container tag of their own and render entirely through lib/ui/page-header.ts, lib/ui/cta-panel.ts, and lib/ui/site-footer.ts, so an app-only glob would cover those routes vacuously.

Running the current check over that widened set finds zero imbalances, but only after extractHtmlTemplates is fixed. As written it reports three false positives (components/doc-search.ts <div> 6/5, plus two files in the gitignored @webjsdev/ui mirror), because its flat ${ / } counter is desynchronized by a bare object literal inside a hole. website/modules/ui/components/dialog.ts L550 has the shape, ${buttonClass({ variant: 'outline' })}, whose } drops the depth to zero early, so the scan ends at the next backtick and the rest of the literal is silently dropped. Fix the extractor first, then widen. So there is no violation backlog to triage and no allowlist to write.

The extractor fix is worth having on its own. Replacing the flat counter with two mutually recursive scanners (template text, hole expression) also stops counting hole SOURCE as markup, which is what a <div inside a JS string in a hole was doing. Such a string renders as escaped text, never as an element, so counting it was always wrong.

Rejected:

  • An allowlist that shrinks over time. The widened set is already clean, so the allowlist would be empty and encode nothing. The repo's other health guards (test/repo-health/site-seo-tags.test.mjs, test/repo-health/gitignore-webjs-depth.test.mjs) assert the invariant outright with no per-file waiver, and this one should match.
  • A site-wide glob over every website/**/*.{js,ts} holding an html template. website/modules/ui/components/ is gitignored (website/.gitignore L7) and regenerated by website/scripts/copy-registry.mjs from packages/ui/packages/registry/, so a failure there is not fixable in website/ and the directory does not exist until npm run pretest runs. website/lib/docs-llms.server.ts reports <pre> 1/0 from a regex in its markdown pipeline, not from markup.
  • Metadata routes and route handlers (app/sitemap.ts, app/robots.ts, app/llms.txt/route.ts, app/llms-full.txt/route.ts, app/api/search/route.ts, app/ui/registry/**/route.ts). They emit XML, plain text, or JSON, so balancing them is meaningless and the docs-llms.server.ts case shows the false-positive shape.
  • Naive brace counting (treat every { as depth, not just ${). It fixes the three false positives, but a literal { in template TEXT (a CSS rule inside <style>, an unescaped code sample) desynchronizes it the other way and can over-run past the literal's real end.
  • Splitting into two test files, one for docs and one for the rest. One failure class, one extractor, one corpus. A split duplicates the extractor or invents a shared helper for two callers.
  • Leaving the file at test/docs/. The corpus is no longer docs-specific.

Steps

  1. In test/docs/docs-pages-well-formed.test.js, replace extractHtmlTemplates (L64-85) with two mutually recursive scanners. scanTemplate(src, i) walks template text from just after an opening backtick, keeps every text character, recurses on ${, and returns at its own closing backtick. scanExpression(src, i) walks a hole with a brace depth starting at 1, skips '...' and "..." strings, recurses into a nested backtick template and KEEPS that nested body (a nested html`...` is real markup), and returns at the } that drops depth to zero. The exported extractor keeps its signature and its concatenated-string return, so nothing downstream changes.
  2. Replace listDocsPages() (L87-93) with a SOURCES table of { pattern, minFiles, minWithBody }, mirroring the APPS table style in test/repo-health/site-seo-tags.test.mjs. Rows: the app render-file glob at 58/55, website/components/**/*.{js,ts} at 6/6, website/lib/ui/**/*.{js,ts} at 4/4. The floors sit a few below today's 62/60, 7/7, and 4/4 so a page landing or leaving does not red the guard, while a glob that collapses back to the docs corpus (44) or to nothing does.
  3. Move the two floor assertions (L103-106 and L127-130) inside the per-row loop so each row proves its own files were FOUND and READ. A single total would let the 4-file row break to zero unnoticed.
  4. Keep CONTAINERS (L39) as is. Keep the test name derived from CONTAINERS rather than spelling the new scope into the string.
  5. Rewrite the file header comment. Keep the incident it was written against, add why the corpus is now three globs, and state what is deliberately outside them with the reason for each.
  6. git mv test/docs/docs-pages-well-formed.test.js test/repo-health/site-pages-well-formed.test.mjs. It sits beside site-seo-tags.test.mjs, which is the same kind of guard (source-level, incident-derived, spanning surfaces), and .mjs matches every other file in that directory. scripts/run-node-tests.js walks test/ and picks up both extensions, so no runner config changes.
  7. Add the guard's scope to website/AGENTS.md under ## Style (L311).

Tests

  • Unit: test/repo-health/site-pages-well-formed.test.mjs is itself the deliverable. It asserts balanced <pre>, <div>, <ul>, <ol>, <table> counts across all three globs, plus the per-row file and template floors.
  • Extractor fixture: add an inline-source case asserting that a template whose hole holds an object literal (${fn({ a: 1 })}<div></div>) extracts to the END of the literal, not to the early backtick. It reds against the current flat counter. Add its mirror, a template whose TEXT holds a bare {, asserting the scan still stops at the real closing backtick.
  • Deliberately-unbalanced fixture: an inline source with one unclosed <div> must produce a failure entry. Verified by hand that the corpus mutation reds too. Deleting one </pre> from website/app/page.ts takes it to open=5 close=4, and the current glob does not look at that file at all.
  • Counterfactual: revert the glob widening alone and the website/app/page.ts mutation stops reding, since the old corpus is website/app/docs/**/page.{js,ts} only. Revert the extractor fix alone and components/doc-search.ts reds at <div> 6/5 against markup that is correctly balanced.
  • Layers: this is a static source check with no runtime, no DOM, and no runtime-sensitive surface, so browser, e2e, and Bun parity do not apply. It runs under npm test.

Doc surfaces

  • website/AGENTS.md, ## Style section (L311). Add a bullet naming test/repo-health/site-pages-well-formed.test.mjs, the three globs it covers, and the fact that shared chrome under lib/ui/ is covered because pages render through it. State the exclusions so nobody adds a metadata route to the corpus.
  • None elsewhere. The change is website- and test-only, so the framework skill at .agents/skills/webjs/, the docs site, the scaffold templates, the MCP surface, root AGENTS.md, and the changelog are all N/A.

Implementation notes (for the implementing agent)

Where to edit

  • test/docs/docs-pages-well-formed.test.js. Current anchors on main: CONTAINERS L39, tagCounts() L48-55, extractHtmlTemplates() L64-85, listDocsPages() L87-93 (the glob is L89), the describe / test pair at L95-96, the pages floor at L103-106, the withBody floor at L127-130, and the failure message at L131-141.
  • website/AGENTS.md under ## Style (L311).
  • Nothing else in website/ changes. The widened corpus is clean.

Landmines / gotchas

  • code-block is NOT in CONTAINERS any more. fix(website): one code-block element owns the grammar and focus stop #1249 (bfa0eb4) swept the roughly 474 docs and gallery samples onto one <code-block> element that owns its own <pre>, and dropped the tag from the list in the same PR. Do not add it back. Its markup is now generated by the element, not hand-authored, so counting it says nothing about a page.
  • The floor assertions are load-bearing. pages.length >= 40 and withBody >= 40 exist because this glob once pointed at a moved directory, matched a single redirect stub with no template, and every check below passed on an empty string. Carry them into the per-row form rather than dropping them, or the same vacuum reopens.
  • Two covered files legitimately yield NO template, so minWithBody must sit below minFiles for the app row. website/app/docs/page.ts is a redirect and website/app/ui/layout.ts renders through a helper.
  • The extraction is per html template literal, not per rendered page. A page whose markup spans helper functions can be legitimately unbalanced within one literal. Measured across the widened corpus this does not happen today, but it is the shape to check first if a new failure looks wrong.
  • app/ui/[name]/page.ts is parameterised and reads the gitignored registry mirror. The guard only reads its SOURCE, so it needs no registry, but npm run pretest (website/scripts/copy-registry.mjs) runs before npm test anyway.
  • Do NOT add metadata routes or route.{js,ts} handlers to the corpus.

Invariants to respect

  • Root AGENTS.md invariant 9: no backticks inside an html template body. Relevant because this test parses those bodies, and the recursive scanner is what decides where one ends.
  • Root AGENTS.md invariant 11 for any prose added.
  • website/AGENTS.md: a code sample under /docs or /ui is a <code-block>, never a bare <pre>.

Acceptance criteria

  • The guard covers every page, layout, and boundary file under website/app/ that renders HTML, plus website/components/** and website/lib/ui/**, not just app/docs
  • extractHtmlTemplates scans holes recursively, so a bare object literal inside a hole no longer truncates the extracted body, and hole SOURCE is no longer counted as markup
  • A counterfactual proves the widening: removing a closing tag from website/app/page.ts reds the guard, and did not before
  • A counterfactual proves the extractor fix: website/components/doc-search.ts reds at <div> 6/5 with the old flat counter and passes with the new scanner
  • Per-glob file and template floors replace the two global ones, so no row can silently collapse to zero
  • The corpus lands with zero violations and zero allowlist entries
  • The file lives at test/repo-health/site-pages-well-formed.test.mjs, moved with git mv so history follows
  • website/AGENTS.md states the guard's scope and its deliberate exclusions

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions