Repository navigation
fix(layout): useAppShellBranding restores the favicon it replaced (objectui#10040) - #10372
Conversation
The effect already scopes its document.title write to the mount (captures the previous title, restores it on cleanup). Its other branded write, the icon link's href, had no restore, so a branded favicon outlived the shell wherever nothing else re-applied one. Capture the icon link element and its href ATTRIBUTE (not the resolved property, which reads an empty href back as the page URL) when the effect writes branding.favicon, and put it back in the same cleanup (removing the attribute when there was none). No favicon, or no icon link, writes and restores nothing. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
Beside the title-restore pins: the branded favicon is written while mounted, the previous href attribute comes back on unmount, an A to B switch restores the pre-A icon, no favicon means no write in either direction, and the attribute bytes (empty, absent) are what is restored. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract reviewServed-tier: Rendered by an isolated review subagent spawned by the ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #10040
Clause-②: no
What changed
useAppShellBranding(packages/layout,AppShell.tsx) now scopes its favicon write to the mount, the same way objectui#8637 scoped itsdocument.titlewrite:branding.faviconis set and the page has an icon link (#favicon, elselink[rel="icon"]), the effect records the link element and itshrefattribute, then writes the branded URL exactly as before;setAttribute), or removes it when the link had none;favicon, or no icon link: nothing is written and nothing is restored. The hook still never creates an icon link.Why the attribute and not
link.href: the property is the URL resolved against the document, so the console's own icon link (href=""inapps/console/index.htmluntil an operator favicon exists) reads back as the page's URL. Restoring through the property would point the icon at the page itself. Measured in happy-dom (the environment-control test) and by ablation leg 2 below.Dispatch mechanism assumptions A1–A4, measured
content/docs/layout/app-shell.mdxsays so), so there is nothing to remove; ablation leg 5 pins that it stays that way. The capture has to be the attribute, not the property (above).branding.favicon, the captured link stays null: no write, no restore (pin, and ablation leg 3).FaviconSyncfromapps/consoleas the first sibling under aMemoryRouter(the console mounts it as the first child ofBrowserRouter, ahead of the routes) and the real hook under/apps/:appName/*, withgetFaviconUrlmocked, logging every write to the icon link. Order within one navigation commit: shell cleanup (restore), thenFaviconSync's effect, then the shell's effect (capture + branded write)./apps/ashows A;/apps/brestores""then writes B;/apps/plainends"";/homeends"". On the branch point86982ace0:/apps/plainstays B and/homestays A, which is the card's defect reproduced at DOM level.86982ace0: the restore puts back O, which is whatFaviconSyncwrites next anyway./apps/ato/apps/a/page-2) makesFaviconSyncwrite O over the branded icon, on both trees. Reported to the seat as an out-of-scope finding, not addressed in this PR.Tests
New:
packages/layout/src/__tests__/app-shell-branding-favicon-restore.test.tsx, beside theapp-shell-branding-title-*pins: 9 tests (1 environment control, 8 pins).Ablations, each a trap-armed mutation with a blob-hash restore proof. The tests import
../AppShellsource directly, so nodist/is on the resolution path.AppShell.tsxset to the branch point86982ace0link.hrefpropertyfaviconpin), 8 passedEvery pin is red in at least one leg. The two that are green on the branch point (write while mounted; no
faviconmeans no write) pin existing behaviour and go red in legs 4 and 3. Every restore leg ended with the file's blob equal to the HEAD blob andgit diff HEADempty.Local verification at
d10a9ad9dpnpm --workspace-concurrency=2 --filter "@object-ui/layout^..." build: exit 0pnpm --filter @object-ui/layout type-check: exit 0;tsc -p tsconfig.test.json --listFilesincludes the new test filepnpm exec vitest run packages/layout/ apps/console/src/__tests__/tabTitleAfterNavigation.test.tsx: 25 files, 245 tests passedcheck-changeset-presence,check-changeset-no-major,check-changeset-overwrite,check-changeset-claims,check-new-cross-file-line-citations(0 new),check-control-bytes,check-test-path-roots,check:doc-example-readers,check-vi-mock-specifiers,check-vi-mock-inherit,check-changeset-fixed: all exit 0;check-governed-queue-guard --teston the three paths: NOT GOVERNED--no-inline-config --format json): 2 files, 0 errors, 1 warning (react-refresh/only-export-componentson theuseAppShellBrandingexport, present on the branch point too). Narrowing stated: the population is the rooteslint.config.jsblock for**/*.{ts,tsx}; the resolved config has emptyparserOptions(no type-aware linting) and no custom rule reads other files, so this diff cannot move an untouched file's verdict. The repo-widepnpm lintis CI's.Pending changesets that name
AppShell.tsx/useAppShellBranding(the 4830, 6871, 6872 and 8637 ones) were re-read: none is made false by this diff.Acceptance notes
FaviconSync's route-keyed re-apply writes the operator favicon over a branded app favicon on every in-app navigation when both are configured; it is the favicon twin of the title race objectui#8637 removed.content/docs/layout/app-shell.mdxsays the colour properties are removed again when the shell unmounts, and does not mention that the title and (now) the favicon are restored too. The sentence stays true; the omission is noted, not addressed here (outside the claimed surface).href;link.type, which the console's boot script andFaviconSyncset by file extension, is left as it was. Observation only.Generated by Claude Code