Repository navigation
fix(app-shell): a record page open reads its record once (objectui#11699) - #11750
Merged
Merged
Conversation
… sends (objectui#11699) The page-record load effect depended on `effectivePage`, `objectDef` and `perms`, three object identities, while it reads from them only whether a page exists and the FLS-gated `$expand` list. Measured on full reloads of a showcase record page, the `$expand` read went out twice in sequence; the second effect run had changed nothing but `objectDef`'s identity (a JSON-equal definition, same `fields` object, handed down again by a host re-render). The effect now keys on `hasPage` and the gated expand list as a string, computed during render from the same inputs. A new object with the same relations, an assigned page replacing a synthesized one, a discarded memo, or a permission answer that leaves the list unchanged no longer re-reads; a changed list still does (objectui#7230's narrowing re-read included). The landed request-count pin now counts the page's own read and adds legs for each identity change, a forced memo discard, and two live controls. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8
…stored input (objectui#11699) The context value's `objects` getter ran `mergeViewsIntoObjects` + `attachInlineSubforms` on every read, and both wrap each object they touch in a new object. Measured on a showcase record page, a host re-render at the same context version handed the record page a JSON-equal definition as a new object, which re-ran its record read. The composed list is now cached outside React, in a module-local WeakMap keyed on the two stored arrays it is built from (`entry.items` for `object` and `view`). Every write to a cache entry replaces that array and none mutates it in place, so the same arrays give the same list and a replaced array gives a new list carrying the change. Both exported helpers keep their signatures. The new pin covers same-version re-reads, another type landing, replacing either stored array (live controls), and a forced discard of the context value memo. The record page pin's discard leg now arms before mount. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8
…out any (objectui#11699) Type-only: the composed-objects cache takes `unknown[]`, and both pins' `useMemo` proxies use a local loose type instead of `any`. No runtime change. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8
… read (objectui#11699) Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8
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
|
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 #11699
Clause-②: no
This is the card's remaining half. The concurrent half landed in PR objectui#11745. On open, the record page sent its own
$expandread of the record (GET /api/v1/data/OBJ?populate=…) twice, one after the other. It now sends it once. Measured on a real stack below. Session:https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8.What this does
Both sides of the seam, per objectui AGENTS.md #10. The producer side is covered by the claim amendment on the card (comment
6026121402).RecordDetailView's record-load effect keys on what the read sends. It depended oneffectivePage,objectDefandperms, three object identities. Read from the code, the effect useseffectivePageonly as a yes/no guard,objectDefonly asobjectDef?.fieldspassed tobuildExpandFields, andpermsonly throughisLoadedandcheckField. It now depends onhasPage(a boolean) andpageRecordExpand, the field-level-security-gated expand list as a JSON string. Both are computed during render from the same inputs. The objectui#7230 gate is unchanged in substance, and its rationale moved with the code: an unanswered policy filters nothing, and an answer that narrows the list re-reads the record without the denied relation.MetadataProvider'sobjectslist keeps its identity. The context value'sobjectsgetter ranmergeViewsIntoObjects+attachInlineSubformson every read, and both wrap every object they touch in a new object. The composed list is now cached in a module-localWeakMapkeyed on the two stored arrays it is built from (entry.itemsforobjectand forview). The same arrays give the same list and entries; a replaced array gives a new list carrying the change. Neither helper's signature changes, and nothing is exported.Measurement
Environment: objectstack
mainat60ccda5a, the showcase app (objectstack dev --seed-admin --fresh, own port, temporary SQLite). The console is a productionvite buildof this worktree withVITE_BASE_PATH=/, served byvite previewwith/apiproxied to that backend. Chromium headless 1440x900, seeded admin. One warm-up visit, then 4 full reloads of ashowcase_taskrecord page per build, counting requests with Playwrightrequestevents and waiting 6 s afterload. Both builds render the record: the page'sh1reads the task's title.055d350(instrumented)c9324d2GET /api/v1/data/showcase_task?populate=…&top=1&filter=…(the page's own read)GET /api/v1/data/showcase_task/ID(the details block's read, shared since PR objectui#11745)The
populateURL (expansion and filter) is byte-identical between the two builds.Which dependency changed between the two reads (the order's mechanism hypotheses, as measured)
The base build carried temporary instrumentation. In the effect, it logged which dependencies differed from the previous run. In the provider's getter, it tagged each object handed out with the getter read and the context
versionthat produced it, and whether it was the stored item. It also logged each metadata type as it landed. The instrumentation was reverted withgit checkout HEAD --before any fix was written:git diff HEADempty, 0 markers, blob equal toHEADfor both files.objectDef,perms, or both:objectDefonly, on 4 of 4 reloads. The second run's changed set was exactlyobjectDef.perms,effectivePage,objectName, the record id,dataSourceand the invalidation nonce were the same values, andobjectDef.fieldswas the same object. The new definition was JSON-equal to the old one. The policy had loaded before the page mounted, and the assigned record page (showcase_task_detail) had landed before the effect's first run, so neither moved here.$expand: holds.RecordDetailView.expandFls-7230.test.tsxis unchanged and green. The new pin adds the case that file does not cover: a policy that arrives after the read and denies a relation reads the record again without it.MetadataProvider'sobjectsgetter at the same contextversion(7), from different getter reads, and neither was the stored item. A host re-render re-read the getter and got a new wrapper for unchanged metadata.Before keying the producer cache on identity, every write to a stored
object/viewarray was read:.filteron a by-name invalidation,[]on a whole-type invalidation, and theapp-only session seed. The provider has no in-place mutator on those arrays. Its only.pushtargets a local map insideattachInlineSubforms.readType,getItemsByTypeandensureType's promise. A per-file census of the 47 non-test files that read them found 0 in-place mutators (sort,push,splice,reverse, index orlengthassignment) on any name bound to a metadata list. A synthetic positive control was flagged 2 of 2..objects.MUTATOR() found 0 hits, with a control flagged 2 of 2.app-shell,reactandplugin-detailsources: 0 hits, with a control flagged 1 of 1.Nothing re-checks this census. The cache's own comment says so (AGENTS.md #9).
Tests
packages/app-shell/src/views/RecordDetailView.recordOpenRequests-11699.test.tsx(the landed pin, extended; 7 tests). The record-open count now includes the page's own$expandread: once, with both relations. New cases, each delivering an identity change the old dependency list re-read on, and counting that read:useMemos overobjectDef(marker-scoped proxy, as inplugin-list'sListView.discardedExpandFieldsMemo.test.tsx), armed before mount;packages/app-shell/src/providers/__tests__/MetadataProvider.composedObjectsIdentity-11699.test.tsx(new, 5 tests). It mounts the real provider over an adapter double that serves a new array per fetch:useMemokeeps the list. The case first shows the discard reached the provider: the context value IS a new object.useMemobut notuseCallback. That is a measured choice; see Acceptance notes.Runs (from the worktree root, through the shared verify lock):
All at
be32eadunless named otherwise.pnpm exec vitest run --maxWorkers=2 packages/app-shell/:Test Files 1052 passed | 1 skipped (1053),Tests 10301 passed | 9 skipped (10310), exit 0. The log carries happy-domNetworkErrortraces from iframe navigations tolocalhost:3000app URLs. They come from a preview test and vitest does not count them.apps/consoletest files that read the metadata provider oruseMetadata(the producer change's consumers outside the package):Test Files 17 passed (17),Tests 184 passed (184), exit 0.Test Files 3 passed (3),Tests 18 passed (18)(at6892ddb; the two later commits are type-only and the changeset).pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json): exit 0, 0error TS, after the closure buildturbo run build --filter=@object-ui/app-shell^...(28 successful, 28 total).tsc -p tsconfig.test.json --listFilesincludes both new test files (2 of 2).pnpm exec eslint --format jsonon the four touched files: 4 files, 0 errors. Both touched sources have the same warning count as on055d350(RecordDetailView.tsx110,MetadataProvider.tsx52), and both test files have 0. This is a declared narrowing to the touched files:eslint.config.jssets no type-aware parser options (projectService/parserOptions.project), so this diff cannot change a verdict on an untouched file. The repo-widepnpm lintis left to CI.Ablations
Each leg mutated committed
HEAD(be32ead) through objectstackscripts/ablation-replace.mjsin WRAP mode. Each anchor hit once and the blob changed. Each restore was proven by blob ==HEADand an emptygit diff HEAD. The mutated files are imported by relative path from the pins, so nodistis involved. Directions were predicted before the runs, and each leg came out as predicted.[effectivePage, objectName, pureRecordId, dataSource, objectDef, recordInvalidationNonce, perms]). 4 red, exactly the four identity cases, withto have a length of 1 but got 2/but got 3. 9 green: both live controls, the count test, and all six tests in the 7230 pin.Gates (exit codes captured before any pipe)
At
be32ead:check:changeset-claims0 (no pending changeset names a file this change touches; this changeset does not negate its own package) ·check:pending-changeset-literals0 ·check-changeset-no-major0 ·check-changeset-presence0 (4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)) ·check-changeset-overwrite0 (1 changeset(s) added, 0 modified, 0 deleted) ·check:control-bytes0 ·check:test-path-roots0 ·check:new-line-citations0 (VERDICT new-cross-file-line-citations: 0 new citation(s)) ·check:vi-mock-specifiers0 ·check:vi-mock-inherit0 ·check:vi-mock-override-shape0 ·check-governed-queue-guard.mjs --testover the 5 paths:NOT GOVERNED, exit 0.NOT MEASURED:
check:readme-exports, reason: no export changes and no README changes. The i18n gates, reason: no locale pack changes. The fullpnpm testfarm and the repo-widepnpm lintare left to CI.Declaration delta
No export, prop, type member or language-pack key changes.
composeObjectsand itsWeakMapare module-local.mergeViewsIntoObjectsandattachInlineSubformskeep their exported signatures. The changeset declares@object-ui/app-shell: patch(.changeset/11699-record-load-once.md), with the measured before and after. Neither the package README norcontent/docsdocumentsuseMetadata/MetadataProvider, so no doc changes.Acceptance notes
objectslist). The provider's preview-mode effect listsbump, auseCallback, as a dependency. After mount, any re-run clears the whole metadata cache. In the provider pin's first draft, a forced discard of everyuseCallbackemptiedobjectsthis way. React does not discard on its own in this tree, so this is dormant. Recorded on the card, not filed.refresh()after a publish or install. The record page no longer re-reads on it, because it keys on data. Other consumers keyed on definition identity may recompute. Not measured here.RecordDetailView.expandFls-7230.test.tsxkeeps its header line "Stable stub identity —permsrides the record-load effect's dependency list". After this change, the gated list rides it instead. The order asked for that file unchanged, so the line stays as it is.runtime/configandget-sessiongo out twice per reload;sys_user_preferenceis PATCHed twice (objectui#11678). Twosecurity/explainPOSTs per reload in both builds; their bodies were not compared here.Generated by Claude Code