Uh oh!
There was an error while loading. Please reload this page.
fix(permissions): give usePermissions() a return identity React cannot discard - #6819
Merged
Merged
Conversation
…t discard `usePermissions()` cached its return in a `useMemo` keyed on `[ctx]`, and both of its branches build a fresh object — an object literal with no provider, a spread of `ctx` with one. `useMemo` carries no semantic guarantee: React may discard the cache and recompute even when `[ctx]` compares equal, which moves the identity while every permission it carries is unchanged. Consumers name that identity in dependency arrays (13 arrays across 6 files), so a discard alone re-ran `ListView`'s data-fetch effect and `DetailView`'s gatedSchema. The decoration becomes a plain function of `ctx` instead: one object per context value, held in a module-level `WeakMap` React has no say over, plus one shared frozen constant for the no-provider answer. What the value is, and when it changes, are both unchanged. 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-sales
marked this pull request as ready for review
August 29, 2026 22:15
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#6724
usePermissions()cached its return in auseMemokeyed on[ctx], and both of its branches build a FRESH object — an object literal when no provider is mounted, a spread ofctxwhen one is.useMemocarries no semantic guarantee: React may discard the cache and recompute even when[ctx]compares equal, handing the caller a new identity while every permission it carries is unchanged. Consumers name that identity in dependency arrays, so a discard alone re-ranListView's data-fetch effect (an extradataSource.find) andDetailView's gatedSchema. Same family as #6018 / #5976 / #6591 / #6592 / #6697.Measured first, because the card asked two questions
1. Is the extra fetch observable today? No — and that changes what this is. The card reasons from React's documented licence, not from a reproduction, so I went looking for one. On React 19.2.8 (this repo's pin) the cache is not discarded spontaneously: 51 re-renders with no provider, 51 with a provider, and 42 under
StrictModeeach returned exactly ONE identity. There is noActivity/Offscreen subtree anywhere in the repo either — that is the documented case where React really does throw memo caches away. So this is a latent hazard, not a bug reproducible from user actions: a correctness dependency resting on a licence React has not yet exercised here. The only way to exercise it is to force it, which the pin does with the module-level discard proxy from #6697's pins.2. What do the consumers actually need — identity, or the values? Identity, and there is no narrower option available. What they read off this object is the VERDICT FUNCTIONS —
perms.checkField(objectName, field, 'read'),perms.can(objectName, 'update')— over an OPEN set of field names, and those flatten to no fixed list of primitives the way #6592'sdataConfigmembers did (provider,object,items). Measured spread of the by-identity dependency:packages/plugin-list/src/ListView.tsxpackages/plugin-detail/src/DetailView.tsxpackages/plugin-detail/src/RelatedList.tsxpackages/plugin-form/src/ObjectForm.tsxpackages/plugin-form/src/ModalForm.tsxpackages/plugin-grid/src/ObjectGrid.tsx13 arrays across 6 files. Re-keying each on primitives is not available (no primitive expresses "which fields are readable"), and it would be 6 files of #6592-style surgery to fix one hook. So the by-identity dependency at the consumers is the correct shape and stays; the fix is at the hook, where the identity is made trustworthy.
The fix
The decoration stops being a React cache and becomes a plain function of
ctx:WeakMapReact has no say over (keyed weakly, so the entry dies with the provider's value);That is strictly stronger than the memo it replaces: the identity is now stable across every component reading the same provider, not just across one component's re-renders. A new context value still yields a new identity, on purpose — that is a real permission change and every consumer must see it.
Two designs were tried and one was rejected on measurement: a
useRefcache keyed onctxworks, but it reads and writesref.currentduring render andpnpm --filter @object-ui/permissions run lintreported threereact-hooks/refswarnings for it. #6745 and #6797 are open findings on exactly that smell in published hooks, so shipping three more of it here would have been filing my own next card. TheWeakMapcosts no hook at all and lints clean.No
ListView.tsxedit — the fix is entirely insidepackages/permissions, so the #6800 / #6723 merge order is unaffected by this PR.Tests
packages/permissions/src/__tests__/usePermissions.discardedIdentity.test.tsx, 7 cases. The discard is forced at the module level, not withvi.spyOn(React, 'useMemo'): the hook reaches its React bindings through a frozen[object Module]namespace that spying cannot patch, which would leave the pin unfalsifiable. First case is the canary proving the proxy reaches the same binding the hook uses.Both ablation directions, run against the committed implementation (
git checkout HEAD -- ...restore, blob hashes compared each leg,git diff HEADempty after each restore):usePermissions.tsfrom26896c689(blob9c74b56, on-disk check:return useMemo(present,NO_PROVIDER_PERMISSIONSabsent): 4 failed | 3 passed, the two discard pins failing withexpected 2 to be 1on the identity count. Restored blob247907b, tree clean, 7 passed.canwas mutated to() => true(anchor matched exactly once; injected string count 1 to 2): 2 failed | 5 passed, the values case failing on the deep-equal of all 17 answers. Restored, 7 passed.The values pin is green on BOTH the pre-fix and post-fix implementations, which is the point: this change moves no permission value. The object still spreads
ctxby identity (asserted member by member), still derivescan/cannotfromctx.check, and the documented no-provider fallbacks —isLoaded: false,userId: null(#5683),systemPermissions: undefinedwithhasCapabilitiesfail-open (#4656) — answer exactly as before.No existing test moved. Not one existing file is edited by this PR, and the suites of every consumer named above pass unchanged on
8a3d94d5e:The one new lint warning is
importOriginal(any)in the pin, matching the 16 files that already spell it that way — a precise module type makes the patcheduseMemo's deps parameterDependencyList, which the proxy signature cannot satisfy (measured:error TS2345).check:readme-exportsis unrelated-prerequisite-not-met on this tree (it wantspackages/react/distbuilt); it reads no file this PR touches.Filed, not folded in
#6813 — both permission providers build their context value in a
useMemotoo (PermissionProvider.tsxline 124,MePermissionsProvider.tsxline 285). A discard there movesctxitself, which is the key this fix caches on, so the chain is discard-immune at the hook but not yet end to end. Same family, one link up; #6724's scope-lock says file it.Generated by Claude Code
Generated by Claude Code