Uh oh!
There was an error while loading. Please reload this page.
feat(app-shell): render global:search and global:notifications instead of the placeholder - #6838
Merged
os-sam merged 1 commit intoAug 30, 2026
Conversation
…stead of the placeholder Both are first-class `PageComponentType` members that the 2026-08-26 maintainer ruling on objectstack#12183 kept declared once objectstack#13117 evidenced both data sources shipped. Nothing registered a renderer, so a page that authored either drew the literal "Component Placeholder" scaffold. Neither block adds a data layer: - `global:search` mounts `useRecordSearch` (the hook the command palette and the full-page search results already use), which prefers the adapter's `searchAll` (`GET /api/v1/search`) and falls back to the per-object fanout. - `global:notifications` mounts `InboxPopover` over the shared inbox feed — the bell ADR-0012/ADR-0030 defines. The header bell's inbox wiring moves out of `AppHeader` into `useInboxBell`, so both bells cut from ONE read with ONE optimistic read overlay (#4225 / #4316). Both registrations publish no `inputs` (`ComponentPropsMap` declares an empty shape for each) and use `skipFallback: true` so neither claims a bare key. Fixes#6757 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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-sam
marked this pull request as ready for review
August 30, 2026 03:36
Uh oh!
There was an error while loading. Please reload this page.
os-sam
deleted the
claude/issue-6757-global-search-notifications-renderers
branch
August 30, 2026 04:01
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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#6757
Two spec
PageComponentTypemembers had no renderer, so a page that authored either drew the literal "Component Placeholder" scaffold. The 2026-08-26 maintainer ruling on objectstack#12183 kept both declared once the readiness read in objectstack#13117 evidenced both data sources shipped, which left the renderer as the remaining half.What this adds — two mount points, no new data layer
global:searchmountsuseRecordSearchfrom@object-ui/react— the same hook the Command-K palette (chrome/CommandPalette.tsx) and the full-page results view (views/SearchResultsPage.tsx) already use. It prefers the adapter'ssearchAll(GET /api/v1/search, cross-object hits with title/snippet/record) and inherits that hook's per-object fanout fallback for adapters without it. Scope is the metadata provider's searchable object set, which is the hook's documented default when noobjectNameswhitelist is passed; a page block has no app-nav context of its own the way the palette and the results page do.global:notificationsmountsInboxPopover— the bell ADR-0012 / ADR-0030 define ("the bell readssys_inbox_message") — over the shared inbox feed.The one refactor, and why it is not optional
The header bell's inbox wiring (rows, badge addends, and the three mark-read paths) moves out of
AppHeaderinto a newhooks/useInboxBell.tsthat both bells mount. Copying it into the new renderer would have re-opened the two defectssharedUserFeedsclosed — #4225 (two owners of one read issuing it twice per page) and #4316 (two derivations of read-state disagreeing, so the bell showed zero unread while Home listed five of the same rows as waiting). With one hook there is no second read and no second optimistic overlay, so a bell in the header and a bell an author declared on a page have no representable state in which they disagree about a row.AppHeaderloses 130 lines and gains 24; nothing else about it changes.Contract shape
Both registrations publish no
inputs.ComponentPropsMapdeclares an empty shape for each member ("declares no props at all" is the recorded intent, and it was re-derived here against the installed@objectstack/spec@17.2.0rather than taken from the card), so declaring even aclassNameinput would advertise an authoring key the contract rejects by name — the forward direction ofapps/console/src/__tests__/registry-inputs-spec-parity.test.tsis a vice on exactly that move. Both useskipFallback: true, so neither claims the baresearch/notificationsregistry key.No spec-side change. This also does not change the Studio page palette:
PALETTE_EXCLUSIONSstill recordsglobal:notificationsas a shell singleton, which is a palette decision about authoring ergonomics and independent of whether a declared type renders. The eager palette placeholder forglobal:searchincomponents/renderers/placeholders.tsxstays too — it is the fallback for a host that embeds@object-ui/componentswithout app-shell, and because these modules import that package its registration always runs first and this one overwrites it. The search ablation below measures that ordering rather than assuming it.Verification — all at
6e602ca4eRun from the repo root, per AGENTS.md; the file that actually executed is named in every line below.
Targeted suite, five cases, green:
The assertions are content, not absence — a page declaring each member is rendered through
SchemaRendererover a fake adapter, and each case asserts DOM that had to travel the block's data path: the search box plus thesearchAllhit "Wayne Enterprises" linking to/apps/crm/crm_account/record/a1, and the bell plus the badge "2" folded from two unreadsys_inbox_messagerows joined against their receipts, plus both row titles once the popover opens. "Not the placeholder" is kept only as a second, weaker line, because an empty render is also not the placeholder.Ablation, per member. Each leg mutated one registration call on disk, proved the mutation landed by anchor counts and a
git hash-objectblob comparison, ran the suite, restored withgit checkout HEAD -- ...against an absolute path, and proved the restore by blob equality plus an emptygit diff HEAD. No rebuild leg: this repo's vitest aliases every workspace package to itssrc, and the suite imports the renderers by relative path, so nothing resolves throughdist/.global:searchbb0037ddto8747ef02global:notificationse960379btode330962Restored state is byte-identical in both legs (
bb0037dd...ande960379b...back to their HEAD blobs,git diff HEADempty), and the suite is 5/5 green again on the restored tree.The asymmetry is worth stating rather than smoothing over: ablating
global:searchleaves the namespace-registration case green, because the eager palette placeholder still answersComponentRegistry.get('global:search')truthily. That is precisely why the render assertion is the load-bearing one for that member and the registry probe cannot stand in for it.Other checks, all on the same tree:
pnpm exec vitest run packages/app-shell/— Test Files 575 passed (575), Tests 5635 passed, 1 skipped. No existing test moved: theAppHeaderrefactor is behaviour-preserving, and the three suites most exposed to it (AppHeader.inboxVariant,sharedInboxFeed.twoSurfaces,sharedInboxFeed.transient404) pass unedited.pnpm exec vitest runoverapps/console/src/__tests__/registry-inputs-spec-parity.test.ts,public-contract.test.ts,preview-samples-registry-resolvable.test.ts,record-block-record-reach.test.tsx,packages/components/src/__tests__/palette-placeholder-blocks.test.tsx,packages/cli/src/utils,packages/app-shell/src/views/metadata-admin/previews— Test Files 49 passed (49), Tests 657 passed. The parity pin needed no edit: its registry graph imports@object-ui/componentsand the console'sregister-plugins, not app-shell, so neither member enters its derived sets.pnpm --filter @object-ui/app-shell type-check—tsc --noEmit && tsc -p tsconfig.test.json, exit 0. Confirmed with--listFilesthat it covers all four new or changed files including the test, so this is a measurement and not an empty one.pnpm run check:side-effects-array— this one caught a real defect: both new modules register at load time and were missing from app-shell'ssideEffects, which a bundler would have dropped silently. Added, and the gate now reads "names exactly the 12 module(s) that register at load time".check:control-bytes,check:phantom-deps,check:self-import,check:i18n-keys,check:i18n-drift,check:vi-mock-specifiers,check:element-data-source-declaration,check:doc-types,lint:coverage,type-check:coverage,check-changeset-presence,check-changeset-no-major,check-changeset-fixed.node scripts/regenerate-known-schema-types.mjs— "OK ... matches the registry derivation", no diff. Both spellings were already listed via the placeholder vocabulary, verified rather than assumed.eslint .for the whole app-shell package, 1020 files, 0 errors, 2795 warnings (file count and counts read from--format json, population from eslint's own config rather than a hand-picked file list). The new files contribute sixno-explicit-anywarnings of the same class the siblingrecord-approvals-renderer.tsxalready carries. Repo-widepnpm lintis left to CI, and that narrowing is sound here because this config enables no type-aware linting (noparserOptions.project, noprojectService), so an edit inside app-shell cannot move a verdict in an untouched file of any other package.Not measured locally, and reported as such rather than as passes:
check:eager-closureandcheck:sdui-registration-pinsboth exit 2 with "no console build to weigh" and describe themselves as a broken gauge in that state. Both needapps/console/dist, i.e. a full console build, which CI performs. The specific hazard the second guards — a wrongsideEffectsarray dropping registrations at bundle time — is the onecheck:side-effects-arraynow covers green.check:readme-exportsexits 1 for the same class of reason ("its type entry./dist/index.d.tsis not on disk -- runpnpm buildfirst"), across READMEs in five packages this branch does not touch.Files
packages/app-shell/src/views/global-search-renderer.tsx(new)packages/app-shell/src/views/global-notifications-renderer.tsx(new)packages/app-shell/src/hooks/useInboxBell.ts(new — extracted fromAppHeader)packages/app-shell/src/views/__tests__/global-page-blocks.render.test.tsx(new)packages/app-shell/src/layout/AppHeader.tsx,packages/app-shell/src/index.ts,packages/app-shell/package.json.changeset/6757-global-search-notifications-renderers.mdnav:menuandapp:launcher(objectui#6661, Phase 1) are not addressed here — out of scope for this card, and that card had not landed when this was written, so the "block registry the Phase-1 members used" does not yet exist in the tree; these two follow the establishedComponentRegistry.register(bare-name, renderer, { namespace, skipFallback })plus barrel side-effect-import pattern thatrecord:approvalsandrecord:attachmentsuse in this same directory.Generated by Claude Code
Generated by Claude Code