fix(website): assert served favicons, not hand-written layout markup - #1381
Merged
Conversation
#1379 moved the website's icons onto metadata.icons and updated the repo-health copy of these assertions, but missed this one: the website carries its OWN copy in its own suite, and it regexed the markup that `renderToString(RootLayout(...))` produces. The icons are spliced into <head> by the framework now, so a bare layout render cannot see them and three assertions went red on main. Read the SERVED page instead. That is the more honest assertion anyway: the tags may come from hand-written markup, from metadata.icons, or from an auto-linked app/icon.* route, and a browser cannot tell the difference, so neither should the test. Add the resolve check the repo-health copy grew at the same time, so a head naming a URL nothing answers fails here rather than as a missing tab mark in production.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1380
mainis red on In-repo app tests (website + blog + gallery). That job passed on7867caeeand fails one0622b59(#1379), so the regression is mine.#1379 moved the website's favicons onto
metadata.iconsand updated the repo-health copy of these assertions, but the website carries its own copy in its own suite, and I missed it. That copy rendersRootLayout(...)alone and regexes the output for the markup, so it cannot see icons the framework splices into<head>. The tags are served correctly; only the test's way of looking for them was stale.Fix
Read the SERVED page instead, matching what
test/repo-health/site-seo-tags.test.mjswas changed to in #1379. That is the more honest assertion regardless: a favicon can arrive from hand-written markup, frommetadata.icons, or from an auto-linkedapp/icon.*route, and a browser cannot tell the difference, so neither should the test.Attribute matching is now order-independent. The framework emits
rel, href, sizes, typewhile the hand-written markup usedrel, href, type, sizes, which was a second reason the regexes broke.Also adds the resolve check the repo-health copy grew at the same time: every icon href in the head is fetched and asserted
200. A head naming a dead URL is invisible from the app and only shows as a missing tab mark, which is how this class of defect reached production on gallery.webjs.dev (#1375, #1377).Left alone deliberately: the canonical tests still assert how the value is DERIVED across URL variants (one rendered page cannot show that), and the
favicon.icocheck stays a filesystem assertion since nothing links it on purpose.Verification
( cd website && node scripts/copy-registry.mjs && npx webjs test )→ 471/471, matching the CI job's count.webjs checkandwebjs typecheckclean.Worth recording, because it cost me a wrong read first: running the website suite in a fresh worktree without
node scripts/copy-registry.mjsfails 9 unrelated UI-gallery tests and ~37 typecheck errors, since that step generates the gitignoredmodules/ui/components/**andlib/utils/cn.ts. The issue body carries this note for the next person.Not in scope
These SEO invariants are asserted in two files, which is why #1379 could update one and leave the other red. Collapsing them is a separate call, flagged in #1380 rather than folded in here.