Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell,console): publish resolveHostAppSegment from the package root and delete the console's copy - #4318
Merged
Conversation
…e root and delete the console's copy (#4280) `resolveHostAppSegment` lived in `app-shell/src/utils/appRoute.ts`, and the package published only its `.` export — whose barrel re-exported `./utils` nowhere. objectui#4109 needed the host-app resolution order for the record an internal `/forms/:name` submit creates, could not import it, and shipped a documented local subset (steps 1-2, `null` where upstream falls through). The root barrel now exports `resolveHostAppSegment` alongside the two predicates it is defined in terms of, and `createdRecordPath.ts` keeps only the URL shape. Its app-record type is derived from the resolver's signature rather than re-declared, so the call site cannot drift from it. Semantics converge on the full resolver: an empty openable list with a preferred app keeps that app unchecked, and anything else unresolvable lands on `setup`. A created-record redirect that used to stop on FormPage's in-place confirmation now navigates to the record — the same answer every other record link in the console gives. Part of #4280. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 14:00
Uh oh!
There was an error while loading. Please reload this page.
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
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.
Part of #4280 — the mechanical dedup half only. The card's optional rider (mounting
ConsoleLayouton the internal/forms/:nameroute) is deliberately NOT in this PR and was not ruled; #4280 stays open for that decision.History: #4109 / PR #4279 disclosed this duplication rather than smuggling it, and #4074 is where the resolution order was won.
What was wrong
resolveHostAppSegmentanswers "which app should host a framework-owned, app-INDEPENDENT page for this user", and its docblock argues at length that its resolution order is one hard-won definition. It lived inpackages/app-shell/src/utils/appRoute.ts, andpackages/app-shellpublished only its.export — whose barrel re-exported./utilsnowhere. The definition was therefore unreachable from every consumer outside the package.Something outside the package needed it anyway. #4109 had to name a host app for the record an internal
/forms/:namesubmit creates, could not import the resolver, and shipped a documented local subset inapps/console/src/components/createdRecordPath.ts: steps 1 and 2 only, returningnullwhere upstream falls through further. Two readers of one prose contract (#3367 / #3842) — the next edit to the order lands on one copy.What this PR does
packages/app-shell/src/index.ts— publishesresolveHostAppSegmentfrom the package root, alongside the two predicates it is defined in terms of (appRouteSegment,filterActiveApps). Sourced from the existing./utilssub-barrel, which already re-exported all three, matching the barrel's grouped-named-re-export idiom (./layout,./views,./hooks,./context).matchAppBySegmentand theappStudio*family are deliberately left unpublished — no consumer outside the package, so no surface.apps/console/src/components/createdRecordPath.ts— pure deletion.routeSegment,openableAppsandresolveRecordHostAppSegmentare gone; the module keeps the URL shape and delegates the choice. The app-record type it accepts is now derived from the resolver's own signature (NonNullable< Parameters< typeof resolveHostAppSegment > [0] > [number]) rather than re-declared — the package publishes the function but not its parameter type, and re-stating the field set was itself part of the duplication. The call site cannot drift from the shape the resolver accepts.@object-ui/app-shellminor,@object-ui/consoleminor — never major, per the fixed-group rule).InternalFormRoute.tsxandFormPage.tsxare untouched: the import path did not change andHostAppLikekeeps its name and its meaning.Behaviour delta — deliberate, not incidental
Converging on the full resolver kills the documented divergence, which is what the card exists for. Two cases that answered
nullnow name an app:preferredsetnullpreferred, unchecked (upstream step 3)nullsetup(upstream step 4)nullsetup_packageIdnornamenullsetupThe first row is upstream's judgement that an empty list means "not loaded yet" at least as often as "this user has no apps", and that demoting someone demonstrably rendering inside
/apps/{preferred}/...would reintroduce the defect #4074 removed.User-visible consequence, on the created-record redirect only.
FormPagereads anullpath as "no record page to land on" and answers by confirming the submit in place (setSubmitted(true)) instead of navigating. With the host app now always named, a submit that previously stopped on that confirmation navigates to the record under the resolved app — the same answer every other record link in the console already gives (RecordDetailView,SearchResultsPage,useObjectActions,InterfaceListPage). The write itself was never at stake; only where the user is put afterwards. A user with zero openable apps who previously saw the confirmation now lands on/apps/setup/{object}/record/{id}.The one
nullbuildCreatedRecordPathstill returns means what it always should have: there is no record to point at, because the caller has no object or no id. That is pinned.Verification
The built surface, not just the source.
apps/console's tsconfig carries no path aliases, so itstscresolves@object-ui/app-shellthrough the package'sexportsfield todist/index.d.ts— the published surface. Afterpnpm --filter '@object-ui/app-shell...' build:dist/index.d.ts:23carriesexport { resolveHostAppSegment, appRouteSegment, filterActiveApps } from './utils';, anddist/utils/appRoute.d.ts:80the full signature returningstring;dist/index.js:33carries the same line, so the runtime surface matches the type surface;dist/index.d.tsand re-running the console'stsc --noEmitproduced exactly the predicted red, which is what proves the console resolves throughdistrather than throughsrc:Tests —
pnpm exec vitest run apps/console/src/components/createdRecordPath.test.ts(repo root, per #3378): 16 passed. The wider sweeppackages/app-shell/src/utils/ apps/console/src/components/: 28 files, 491 tests, all passing. Type-check green for both packages (4tscinvocations:--noEmitplus the typetests/node project each). ESLint clean on all three changed files.The suite gained a pin on the package-root export itself, imported from
@object-ui/app-shelland never from the deep path — the deep path resolves either way and would hold the pin green through exactly the regression it exists to catch.Reverse verification, predictions first.
git checkout origin/main -- createdRecordPath.ts). Predicted: exactly the 4 delta cases go red, the 2 root-export pins and the other 10 stay green. Actual:Tests 4 failed | 12 passed (16), each failure readingAssertionError: expected null to be '/apps/.../obj/record/1'. Match.git checkout origin/main -- app-shell/src/index.ts). Predicted: the ESM link fails and the whole file fails to collect, 0 tests run. Actual: the direction held but the failure mode did not — Vite's SSR transform turns a missing named export intoundefinedat runtime rather than a link error, so the file collected and 15 of 16 failed withTypeError: resolveHostAppSegment is not a function(the 16th is the no-object/no-id case, which returns before reaching the resolver). Recorded as measured rather than as predicted.Reverting was done with
git checkoutagainst a committed HEAD throughout — nevergit stash, which shares one stack across every worktree of the repo (objectui#3430).One piece of drift this PR creates and does not fix
FormPage.tsx(around line 123) justifies itsFORM_RECORD_ID_PARAMliteral by pointing at "the same unreachabilitycreatedRecordPath.tsdocuments forresolveHostAppSegment". Its own claim stays true — the root still does not re-export./urlParams— but the comparison it draws is now stale, becausecreatedRecordPath.tsdocuments no such unreachability any more. Left alone deliberately: FormPage was fenced off for this dispatch (#4292 had just landed there), and the fix is a one-clause comment edit rather than anything behavioural.Generated by Claude Code