Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-view): gate ObjectView's non-grid query on the object schema - #6462
Merged
Conversation
The non-grid fetch effect built its expand set from `objectSchemaRef.current`,
a ref assigned in the render body, and deliberately omitted `objectSchema` from
its dependency list. That bought one effect run per mount and paid for it with
the expansion, permanently: on that one run the ref was still null,
`buildExpandFields` saw no fields, and the query went out as `{ $top: 100 }`
with no `$expand` at all — and the effect never re-ran on the schema's arrival.
ObjectView hands those rows to the child as `data={data}`, suppressing the
child's own fetch, so every lookup / master_detail / user / tree field in the
six non-grid views it hosts (kanban, calendar, gallery, timeline, gantt, map)
rendered from raw foreign-key ids.
The schema read and the fact that it has SETTLED are now one piece of state,
keyed by object name, and the record query waits on it. The gate is on the read
having settled, not on a truthy schema: a view whose adapter exposes no
`getObjectSchema`, or whose read threw, still queries — unexpanded — rather
than waiting forever.
Measured on this effect rather than inherited from the kanban's, because it has
five more dependencies. Instrumented adapter, schema/find both 30ms, four host
regimes: before, 1 find with no `$expand` ever and one raw delivery to the
child; with `objectSchema` in the deps, 2 finds and TWO deliveries (`raw` then
`expanded`) — a visible two-step paint, since the raw rows settle before the
re-run's cleanup here, unlike on the kanban where they were discarded; gated,
1 find carrying `$expand` the first time and one expanded delivery, with
correct rows landing at the same wall clock as the dependency version.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4qCo-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
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-support-ai
marked this pull request as ready for review
August 26, 2026 01:43
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
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#6419
ObjectView's non-grid fetch effect built its expand set fromobjectSchemaRef.current— a ref assigned in the render body, deliberately kept out of the effect's dependency list so the effect would run exactly once per mount. On that one run the ref was stillnull, sobuildExpandFieldsreturned[]and the query went out as{ $top: 100 }with no$expandat all; because the effect never re-ran on the schema's arrival, it never went out with one either.ObjectViewhands the rows it fetches to the child view asdata={data}, which suppresses that child's own fetch. So every lookup / master_detail / user / tree field in the six non-grid views it hosts — kanban, calendar, gallery, timeline, gantt, map — rendered from raw foreign-key ids.The object schema and the fact that its read has settled are now one piece of state, keyed by object name, and the record query waits on it. The gate is on the read having settled, not on a truthy schema: a view whose adapter exposes no
getObjectSchema, or whose read threw, still queries (unexpanded) rather than waiting forever, and switching objects closes the gate in the same commit rather than sending the previous object's expand set.The measurement — taken on THIS effect, not inherited
The charter required measuring what an extra re-run costs here before adopting #6271's gate shape, because this effect has five more dependencies than the kanban's (
currentViewType,currentNamedViewConfig,activeView,renderListView,refreshKey). Instrumented adapter,getObjectSchemaandfindboth resolving in 30 ms, four host regimes (bare; namedlistViews;viewsprop;viewsprop with a re-rendering parent). Deliveries are the non-empty row arrays handed to the child, tagged with the query that produced them:findcallsorigin/main){$top:100}—$expandnever1 -> ["raw"]objectSchemaadded to the deps[{$top:100}, {$top:100,$expand:[...]}]2 -> ["raw","expanded"]{$top:100,$expand:["owner","account"]}1 -> ["expanded"]The middle row is where this component parts company with the board, and it is why the kanban's numbers could not decide this. On
ObjectKanbanthe unexpanded first response was discarded on arrival —isMountedflipped false before it landed — so an extra re-run bought a wasted round trip and no visible artefact. Here the measured order isschema:settled -> find:settled -> find:issued: the raw rows settle intosetDatabefore the re-run's cleanup, reach the child, and paint. An extra re-run on this effect therefore costs a visible two-step render — every relation field blank (kanban'sisOpaqueId) or a raw id for ~40 ms, then swapping — which is exactly the "duplicate events in child views like the calendar" the removed ref-comment cited. So the measurement supports the gate more strongly here than on the board, for a different reason.What the gate costs is one schema resolution ahead of the query, and this component already issues that read unconditionally on mount — measured
getObjectSchemacalls = 1 in every regime, before and after. Correct, expanded rows land at the same wall clock either way: 66.8-69.1 ms gated vs 68.3-69.3 ms via the dependency list, with half the queries and no wrong paint in between.Two further readings worth recording. The five extra dependencies do not amplify the gate: in the churning regime the gate lowered the query count (4 -> 3) because every run now happens after the resolution settles. And that same regime is the only way
origin/mainever sends an$expand— by the luck of an unrelated parent re-render after the schema landed, not by design.Reverse verification
The pin was run against unmodified
origin/main(ObjectView.tsxrestored fromorigin/main, confirmed on disk by blob hash23c0163f2b6c3c56e514a5f8b4d4f1f00e7365c3, gate line count 0 / ref line count 1). Predicted red; observed red — 6 of 8 failed:Against this branch:
Test Files 1 passed (1)/Tests 8 passed (8). Restored by hash, not by exit code — on-disk hash back to0ed68e6a514739a481c9ecf5d46da364d7803726withgit diff HEADempty for the path.The two tests that pass in both directions are the ones guarding the truthy-gate trap (adapter with no
getObjectSchema; grid path still delegating). They cannot discriminate againstorigin/main, which has no gate at all — they exist so a future truthy-value gate goes red.Ghost-assertion guard. A query count, or an
$expandpresence check, would also pass if the view stopped fetching altogether. So every count is reached only after waiting for a real call; the first test'swaitFortargets the expanded call specifically, so zero fetches times out rather than reading as success; one test asserts rows actually reach the child; and$expandis asserted against the expandable fields derived from the fixture schema through core's ownEXPANDABLE_FIELD_TYPES(all four relation types), in both directions — the four expandable names must be present and the three plain ones absent.Verification
All readings below are from the gate union re-run on the final commit,
eb329847b, and each quotes the gate's own verdict line rather than a shell$?read through a pipe.pnpm exec vitest run packages/plugin-view/->Test Files 24 passed (24)/Tests 231 passed (231)pnpm exec vitest run packages/app-shell/src/views/(app-shell consumes@object-ui/plugin-view) ->Test Files 324 passed (324)/Tests 3085 passed | 1 skipped (3086)pnpm --filter @object-ui/plugin-view type-check-> exit 0, script echoed astsc --noEmit && tsc -p tsconfig.test.json. The dependency closure was built first (pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-view^...' build); without it a fresh worktree reportsCannot find module '@object-ui/core', which is missingdist/*.d.ts, not a type error. Confirmed the new pin file is genuinely in the program rather than excluded:tsc -p tsconfig.test.json --listFileslistsObjectView.expandGate.test.tsxandObjectView.tsx.node scripts/check-changeset-presence.mjs->2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjs->No changeset declares a major bump.node scripts/check-control-bytes.mjs->OK (scanned 5323 tracked text file(s); skipped 85 binary), plus a directgrep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over the changed files: clean.node scripts/check-vi-mock-specifiers.mjs->OK(the new test carriesvi.mockcalls)node scripts/check-phantom-dependencies.mjs,node scripts/check-package-self-import.mjs-> both OKDeclared lint narrowing. Repo-wide
turbo run lintwas not run locally;eslint . --no-inline-configwas run over the whole ofpackages/plugin-view— the complete CI lint unit for the only package this diff changes — giving0 errors, 265 warningsacross 39 files, a count read from eslint's own--format jsonoutput, over the file set eslint's own config selected rather than a set I chose. Nothing outside that package can shift:eslint.config.jsdeclares noparserOptions.project/projectService, so linting here is not type-aware and this diff cannot move the verdict on a file it does not touch. CI runs the full farm regardless.Warning delta on the edited file, measured before/after against
origin/main's blob: 60 -> 59. The one that disappears isreact-hooks/refs, the render-body ref write this removes.react-hooks/exhaustive-depsis unchanged at 3, so dropping the now-falseeslint-disable-next-line react-hooks/exhaustive-depsadded nothing —objectSchemaReadyandobjectSchemaare both listed honestly, and both are load bearing:objectSchemaisnullin two different situations (before the read settles, and after it settles with nothing) and only the first may hold the query.Notes
objectSchemanow readsnullwhile a new object's schema resolves, where before the previous object's schema lingered. The two other consumers degrade cleanly —deriveRecordSurface(null, ...)falls back to its default surface and the overlay label falls back toschema.objectName— and this is the same property that stops a query carrying the previous object's expand set.mergedSorthandstable.defaultSortto the delegated list view UNWRAPPED, while the non-grid fetch path wraps the same value — the #5270 arity split, one branch short #6235 is not addressed here: same fetch path, different defect (sort lowering).objectfetch never injects$expand— same empty-ref read objectui#6419 removed from ObjectView #6453 (ObjectCalendarcarries the identical empty-ref read on its own standaloneobjectfetch) and ObjectView's non-grid fetch re-runs once per parent render when the host passes an inlineviewsarray — measured 4 queries where a stable array gives 1 #6460 (this effect re-runs once per parent render when the host passes a freshviewsarray — measured 4 queries before this PR, 3 after).Draft, per the dispatch: the PM lands it.
Generated by Claude Code
Generated by Claude Code