fix(console): one writer owns the tab title after an in-app navigation (objectui#8637) - #10034
Merged
Merged
Conversation
No behaviour change — the component body, its dependency list and its placement inside `BrowserRouter` are carried over verbatim. Pulling it out of `App.tsx` gives the route-keyed `document.title` writer an importable name, so a browser probe and a test can render it beside `AppShell`'s writer instead of a hand-written replica of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
`BrandingSync` assigned the bare product name to `document.title` from an effect keyed on `useLocation()`, while `useAppShellBranding` assigns the composed "App label — Product name" from an effect keyed on that string. Both fire on the commit that mounts the shell and the composed title wins, so the tab looks right; an in-app navigation moves `location` and not the composed title, so only the route-keyed writer runs and the tab falls back to the bare product name. Reproduced in real Chromium against these two components under a real `BrowserRouter` before this change, and again after. `useAppShellBranding` now owns the title while a shell is mounted: it captures the current title, writes `title` over it, and restores the capture on unmount or when `title` changes. That is what makes one writer enough — the route-keyed write had doubled as the reset that took the app label off the tab when the shell went away. The console component keeps only its favicon sync and is renamed `FaviconSync` to say so. Pins: `tabTitleAfterNavigation.test.tsx` renders the real `FaviconSync` beside the real `AppShell` and navigates, which is the assertion the card says was missing; `app-shell-branding-title-restore.test.tsx` pins the restore half. The source-level writer pin in `app-shell-branding-title-surfaces.test.ts` failed on the restore, as it should have — it now pins both writers by role and right-hand side, keeping its exact count and adding the restore's RHS rather than relaxing anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
The first draft reached the router through a module-scoped handle assigned
from inside a component, which `react-hooks/react-compiler` rejects outright
("Cannot reassign variables declared outside of the component/hook") — the
console's lint run was the only red in the targeted gates. Clicking a `<Link>`
is both lint-clean and closer to what the console actually does when the
sidebar moves between pages of an app.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
Contributor
✅ 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
|
os-tesla
marked this pull request as ready for review
September 19, 2026 18:49
os-tesla
enabled auto-merge
September 19, 2026 18:49
os-tesla
pushed a commit
that referenced
this pull request
Sep 19, 2026
…sories objectui#8637's PR #10034 landed while this branch was reporting, so the base moved. Merge rather than rebase: this branch is pushed and may be checked out elsewhere, and the repo's convention is that history on a pushed branch is never rewritten. The squash merge drops this commit from main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
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
2 participants
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.
Fixes #8637
What was wrong, and how it was measured
Two effects wrote
document.titleon different keys.apps/console/src/App.tsxrenderedBrandingSync, a sibling mounted before the shell insideBrowserRouter, whose effect was keyed onuseLocation()and assigned the bare product name on every route change.packages/layout/src/AppShell.tsx—useAppShellBrandingassigns the composed"App label — Product name"from an effect whose dependency list ends in that string, so it fires when the title changes and not on navigation.ConsoleLayoutis what composes that string.Both fire on the commit that mounts the shell, in tree order, and the composed title wins — which is why the tab looks right and the defect stays hidden. An in-app navigation moves
locationand not the composed title, so only the route-keyed writer runs.The card's PM note ruled that reading the code does not settle this and that happy-dom does not either, so the reading below is real Chromium (the preinstalled
/opt/pw-browsers/chromium, driven by the repo's own Playwright), on the realBrandingSync/FaviconSyncand the realAppShell, under a realBrowserRouterwith clicks on real router links.document.title's accessor was instrumented so every write is recorded in order; without a backendgetProductName()is its default,ObjectOS.Before —
030a675b0plus the verbatim extraction commit:Step 2 is the defect: a single write, the bare product name, over the specific title. Step 1 shows why it hides — the composed title wins the mount. Step 3 is the part a static reading does not give you, and it decided the shape of the fix: the route-keyed writer was also the reset that took the app label back off the tab when the shell went away.
After — this branch:
Step 2 is now zero writes — nothing touches the tab title on an in-app navigation at all. Step 3 is byte-for-byte what it was before, and it is now the shell's own restore rather than a route-keyed reset. The set-restore-set in steps 1 and 4 is StrictMode double-invoking the effect; the capture is taken inside the effect, so it is idempotent.
The probe page and its driver were temporary and are not in this diff.
The fix — one writer, not two careful ones
Triage asked for one writer and said the other should stop writing
document.titleat all rather than writing it carefully.app-shell-branding-title-assignment.test.tsxalready decided which one survives, so the writer that stops is the console's.useAppShellBrandingnow ownsdocument.titlefor as long as a shell is mounted. It captures whatever the tab already said, writestitleover it, and puts the capture back when the shell unmounts ortitlechanges.previousTitlestays null when the hook writes nothing, so a shell with notitlestill leaves the tab alone in both directions.FaviconSync, which is all it does now. It stays keyed on the route, and its docblock says why a title assignment must not come back.BrandingSyncwas first extracted fromApp.tsxverbatim, in its own commit, so the probe and the pin render the component the console actually ships instead of a replica of it. A replica would pin the replica and let the defect walk back in through the real component.Nothing about the forward assignment changed: still wholesale, still no concatenation.
The pin the card asked for
From the card: "the reason this survived is that no test asserts what the tab title is after a navigation. A fix without that pin re-opens the same hole."
apps/console/src/__tests__/tabTitleAfterNavigation.test.tsx— renders the realFaviconSyncbeside the realAppShell, clicks a router link to a second page of the same app, and asserts the tab still reads the composed title. Also pins that leaving the app hands the title back.packages/layout/src/__tests__/app-shell-branding-title-restore.test.tsx— the restore half, at the package that owns it, including the documented cost: the restore replays the captured string unconditionally, so a surface that writes the tab title from inside a mounted shell is overwritten on unmount.Both files open with an environment control, so a vacuous green in a DOM with a no-op
titlesetter is distinguishable from a real one.Control reading. Re-adding the route-keyed title assignment to
FaviconSync— mutation proved on disk by an occurrence count and agit hash-objectcomparison against the HEAD blob, then restored, withgit diff HEADverified empty:The two navigation assertions fail and the other three pass — the pin fails for the reason it exists, not for an unrelated one. Removing the restore from
AppShell.tsxinstead (previousTitleoccurrences 5 -> 1, blobs differ, restored clean) turns 6 assertions red across three files.One existing pin was changed, and it is stricter afterwards
app-shell-branding-title-surfaces.test.tsasserted thatAppShell.tsxcontains exactly onedocument.titlewriter, so a second one could not appear without the wording pin noticing. The restore is a second source write, and that assertion went red — the pin doing its job.What was kept: the assigning writer is still exactly one, its operator is still
=, its right-hand side is still the baretitle, and the total is still an exact count, so a third writer still fails. What was added: the restore's own right-hand side is now pinned too, so the cleanup cannot start composing a title either. The docblock records that this began as "exactly one writer" and why it is now two-by-role.app-shell-branding-title-assignment.test.tsxis untouched — it is not in this diff — and green. It is also the pin that decided this card's direction, so it is worth saying that it passes identically on the defect and on the fix: it never navigates, which is exactly the hole this PR fills.Validation
All run from the repo root with
pnpm exec vitest run PATH; the heavy runs went through the shared verify lock in../objectstack.pnpm exec vitest run packages/layout/ apps/console/— 124 files, 1387 tests passed, exit 0.pnpm exec vitest run packages/app-shell/— the direct consumer that mountsAppShellthroughConsoleLayout: 734 files, 7274 passed, 1 skipped, exit 0.turbo run type-check lint --filter=@object-ui/layout --filter=@object-ui/console— 40/40 tasks, exit 0. Lint is warnings-only on both packages and the counts are the pre-existing ones.check:changeset-presence(green, names this changeset),check:new-line-citations,check:control-bytes,check:test-path-roots,check:unreferenced-sources,check:changeset-claims,check:pending-changeset-literals,check:vi-mock-specifiers— all exit 0.node scripts/check-governed-queue-guard.mjs --testover this diff's 7 paths: not governed, no surface matched.Repo-wide
pnpm lintand the fullpnpm testare CI's run, not narrowed here.Scope
The card's own warning is that "one writer" is not literally achievable, so here is each remaining writer and what was done about it.
apps/console/index.html, inline boot script — writes the bare product name during parse, before React exists. Left alone. Different lifecycle; it is the correct title until a shell with an app label is on screen, and it is now the string the shell captures and restores.apps/console/src/main.tsx— same, immediately beforecreateRoot().render(). Left alone, same reasoning. The pair is objectui#5544's subject, not this card's.apps/console/src/pages/auth/AuthLayout.tsx— already save-set-restore for its host pill, scoped to its own mount. Left alone; it is the same discipline this PR gives the shell, one lifecycle down, and the console's auth routes sit outsideConsoleLayoutso the two never nest.apps/console/src/App.tsx/FaviconSync— the one that stopped writing.Out of the fence and untouched:
pnpm-lock.yaml,packages/app-shell/package.json,packages/plugin-kanban, and machine-localeIntlsites.Acceptance notes
Noted while in these files, not filed, no measurement taken:
useAppShellBrandingsetsbranding.faviconwhile mounted and does not restore it; what puts the runtime favicon back today isFaviconSync's route-keyed re-apply. That re-apply is a no-op whengetFaviconUrl()is falsy, i.e. when no operator favicon is configured — in which case a branded app's favicon would outlive the app. Stated as the condition that would make it a defect, not as a measured one: no browser reading was taken for the favicon path. Whoever next touchesuseAppShellBrandingis looking straight at it.Landing is the dispatching seat's act: this PR stays draft, is not enqueued and has no auto-merge.
Implemented from session
session_018HrVaotisyhgmot9o2MLRq.🤖 Generated with Claude Code
https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
Generated by Claude Code