From d5e79f3660274d2991a0dbcfced3fdc90f7d1933 Mon Sep 17 00:00:00 2001 From: Marshall Han Date: Mon, 20 Jul 2026 19:10:55 +0900 Subject: [PATCH] Recover missing-data fragment reads by refetching the owner once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a fragment reads missing data with no pending operation for its owner — its records were garbage-collected while a React was hidden, or a fragment ref outlived the owner query's retain — the EXPERIMENTAL useFragment implementation renders the partial snapshot: fields the fragment explicitly fetched come back undefined, crashing consumers that trust the schema types. There is no recovery path. Behind a new feature flag ENABLE_MISSING_DATA_OWNER_REFETCH (default off), refetch the fragment's owner query once per owner per environment and suspend on the request instead. force:true plus a unique QueryResource cache breaker guarantee a real network request even when a completed QueryResource entry or an app response cache would short-circuit; only query owners re-execute; a FIFO-capped WeakMap marker prevents request loops when the refetch response is still partial or the transport fails, and clears once the owner query reads back without missing data. All new tests trigger real GC (retain().dispose() with gcReleaseBufferSize 0) — no store internals are stubbed. Co-Authored-By: Claude Fable 5 --- ...aOwnerRefetchTestKeepAliveQuery.graphql.js | 124 ++++++ ...gDataOwnerRefetchTestOwnerQuery.graphql.js | 153 +++++++ ...ataOwnerRefetchTestUserFragment.graphql.js | 71 ++++ ...ragment-missing-data-owner-refetch-test.js | 387 ++++++++++++++++++ .../useFragmentInternal_EXPERIMENTAL.js | 156 +++++++ .../relay-runtime/util/RelayFeatureFlags.js | 7 + 6 files changed, 898 insertions(+) create mode 100644 packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestKeepAliveQuery.graphql.js create mode 100644 packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestOwnerQuery.graphql.js create mode 100644 packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestUserFragment.graphql.js create mode 100644 packages/react-relay/relay-hooks/__tests__/useFragment-missing-data-owner-refetch-test.js diff --git a/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestKeepAliveQuery.graphql.js b/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestKeepAliveQuery.graphql.js new file mode 100644 index 0000000000000..3608cb5d3b0ff --- /dev/null +++ b/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestKeepAliveQuery.graphql.js @@ -0,0 +1,124 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<<9a129e363a4240cd6c038fb2501c160a>> + * @flow + * @lightSyntaxTransform + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ConcreteRequest, Query } from 'relay-runtime'; +export type useFragmentMissingDataOwnerRefetchTestKeepAliveQuery$variables = { + id: string, +}; +export type useFragmentMissingDataOwnerRefetchTestKeepAliveQuery$data = { + readonly node: ?{ + readonly id: string, + }, +}; +export type useFragmentMissingDataOwnerRefetchTestKeepAliveQuery = { + response: useFragmentMissingDataOwnerRefetchTestKeepAliveQuery$data, + variables: useFragmentMissingDataOwnerRefetchTestKeepAliveQuery$variables, +}; +*/ + +var node/*: ConcreteRequest*/ = (function(){ +var v0 = [ + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "id" + } +], +v1 = [ + { + "kind": "Variable", + "name": "id", + "variableName": "id" + } +], +v2 = { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "id", + "storageKey": null +}; +return { + "fragment": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Fragment", + "metadata": null, + "name": "useFragmentMissingDataOwnerRefetchTestKeepAliveQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + (v2/*:: as any*/) + ], + "storageKey": null + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Operation", + "name": "useFragmentMissingDataOwnerRefetchTestKeepAliveQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "__typename", + "storageKey": null + }, + (v2/*:: as any*/) + ], + "storageKey": null + } + ] + }, + "params": { + "cacheID": "12a7f4fcd84cf1b3003b3f7392499515", + "id": null, + "metadata": {}, + "name": "useFragmentMissingDataOwnerRefetchTestKeepAliveQuery", + "operationKind": "query", + "text": "query useFragmentMissingDataOwnerRefetchTestKeepAliveQuery(\n $id: ID!\n) {\n node(id: $id) {\n __typename\n id\n }\n}\n" + } +}; +})(); + +if (__DEV__) { + (node/*:: as any*/).hash = "1a1f85e95c8c0ed8143b68b1ffb39df3"; +} + +module.exports = ((node/*:: as any*/)/*:: as Query< + useFragmentMissingDataOwnerRefetchTestKeepAliveQuery$variables, + useFragmentMissingDataOwnerRefetchTestKeepAliveQuery$data, +>*/); diff --git a/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestOwnerQuery.graphql.js b/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestOwnerQuery.graphql.js new file mode 100644 index 0000000000000..8b8a19aa80854 --- /dev/null +++ b/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestOwnerQuery.graphql.js @@ -0,0 +1,153 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<> + * @flow + * @lightSyntaxTransform + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ConcreteRequest, Query } from 'relay-runtime'; +import type { useFragmentMissingDataOwnerRefetchTestUserFragment$fragmentType } from "./useFragmentMissingDataOwnerRefetchTestUserFragment.graphql"; +export type useFragmentMissingDataOwnerRefetchTestOwnerQuery$variables = { + id: string, +}; +export type useFragmentMissingDataOwnerRefetchTestOwnerQuery$data = { + readonly node: ?{ + readonly $fragmentSpreads: useFragmentMissingDataOwnerRefetchTestUserFragment$fragmentType, + }, +}; +export type useFragmentMissingDataOwnerRefetchTestOwnerQuery = { + response: useFragmentMissingDataOwnerRefetchTestOwnerQuery$data, + variables: useFragmentMissingDataOwnerRefetchTestOwnerQuery$variables, +}; +*/ + +var node/*: ConcreteRequest*/ = (function(){ +var v0 = [ + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "id" + } +], +v1 = [ + { + "kind": "Variable", + "name": "id", + "variableName": "id" + } +]; +return { + "fragment": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Fragment", + "metadata": null, + "name": "useFragmentMissingDataOwnerRefetchTestOwnerQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "args": null, + "kind": "FragmentSpread", + "name": "useFragmentMissingDataOwnerRefetchTestUserFragment" + } + ], + "storageKey": null + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": (v0/*:: as any*/), + "kind": "Operation", + "name": "useFragmentMissingDataOwnerRefetchTestOwnerQuery", + "selections": [ + { + "alias": null, + "args": (v1/*:: as any*/), + "concreteType": null, + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "__typename", + "storageKey": null + }, + { + "kind": "InlineFragment", + "selections": [ + { + "alias": null, + "args": null, + "concreteType": "Date", + "kind": "LinkedField", + "name": "birthdate", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "day", + "storageKey": null + } + ], + "storageKey": null + } + ], + "type": "User", + "abstractKey": null + }, + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "id", + "storageKey": null + } + ], + "storageKey": null + } + ] + }, + "params": { + "cacheID": "ab37d7e0988d76c27d43ebe128c344f4", + "id": null, + "metadata": {}, + "name": "useFragmentMissingDataOwnerRefetchTestOwnerQuery", + "operationKind": "query", + "text": "query useFragmentMissingDataOwnerRefetchTestOwnerQuery(\n $id: ID!\n) {\n node(id: $id) {\n __typename\n ...useFragmentMissingDataOwnerRefetchTestUserFragment\n id\n }\n}\n\nfragment useFragmentMissingDataOwnerRefetchTestUserFragment on User {\n birthdate {\n day\n }\n}\n" + } +}; +})(); + +if (__DEV__) { + (node/*:: as any*/).hash = "8338f0fba26c637a3afff95c70b07479"; +} + +module.exports = ((node/*:: as any*/)/*:: as Query< + useFragmentMissingDataOwnerRefetchTestOwnerQuery$variables, + useFragmentMissingDataOwnerRefetchTestOwnerQuery$data, +>*/); diff --git a/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestUserFragment.graphql.js b/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestUserFragment.graphql.js new file mode 100644 index 0000000000000..879b84b5a518d --- /dev/null +++ b/packages/react-relay/relay-hooks/__tests__/__generated__/useFragmentMissingDataOwnerRefetchTestUserFragment.graphql.js @@ -0,0 +1,71 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<<94259f8788980f0b57387b59a098cbc9>> + * @flow + * @lightSyntaxTransform + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { Fragment, ReaderFragment } from 'relay-runtime'; +import type { FragmentType } from "relay-runtime"; +declare export opaque type useFragmentMissingDataOwnerRefetchTestUserFragment$fragmentType: FragmentType; +export type useFragmentMissingDataOwnerRefetchTestUserFragment$data = { + readonly birthdate: ?{ + readonly day: ?number, + }, + readonly $fragmentType: useFragmentMissingDataOwnerRefetchTestUserFragment$fragmentType, +}; +export type useFragmentMissingDataOwnerRefetchTestUserFragment$key = { + readonly $data?: useFragmentMissingDataOwnerRefetchTestUserFragment$data, + readonly $fragmentSpreads: useFragmentMissingDataOwnerRefetchTestUserFragment$fragmentType, + ... +}; +*/ + +var node/*: ReaderFragment*/ = { + "argumentDefinitions": [], + "kind": "Fragment", + "metadata": null, + "name": "useFragmentMissingDataOwnerRefetchTestUserFragment", + "selections": [ + { + "alias": null, + "args": null, + "concreteType": "Date", + "kind": "LinkedField", + "name": "birthdate", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "day", + "storageKey": null + } + ], + "storageKey": null + } + ], + "type": "User", + "abstractKey": null +}; + +if (__DEV__) { + (node/*:: as any*/).hash = "55efb5537c658de35c05222a4d876973"; +} + +module.exports = ((node/*:: as any*/)/*:: as Fragment< + useFragmentMissingDataOwnerRefetchTestUserFragment$fragmentType, + useFragmentMissingDataOwnerRefetchTestUserFragment$data, +>*/); diff --git a/packages/react-relay/relay-hooks/__tests__/useFragment-missing-data-owner-refetch-test.js b/packages/react-relay/relay-hooks/__tests__/useFragment-missing-data-owner-refetch-test.js new file mode 100644 index 0000000000000..d1882fa35f33a --- /dev/null +++ b/packages/react-relay/relay-hooks/__tests__/useFragment-missing-data-owner-refetch-test.js @@ -0,0 +1,387 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + * @format + * @oncall relay + * @jest-environment jsdom + */ + +'use strict'; + +const useFragment = require('../useFragment'); +const ReactTestingLibrary = require('@testing-library/react'); +const React = require('react'); +const {RelayEnvironmentProvider} = require('react-relay'); +const { + FRAGMENT_OWNER_KEY, + RecordSource, + Store, + createOperationDescriptor, + graphql, +} = require('relay-runtime'); +const RelayFeatureFlags = require('relay-runtime/util/RelayFeatureFlags'); +const {createMockEnvironment} = require('relay-test-utils'); + +const {act} = ReactTestingLibrary; +// $FlowFixMe[missing-export] Not yet exists in the Flow types in OSS +const Activity: $FlowFixMe = React.unstable_Activity; +const Suspense = React.Suspense; + +let environment; +let gqlFragment; +let gqlOwnerQuery; +let gqlKeepAliveQuery; +let ownerOperation; +let keepAliveOperation; +let ownerRetention; +let userRef; + +beforeEach(() => { + RelayFeatureFlags.ENABLE_MISSING_DATA_OWNER_REFETCH = true; + + // gcReleaseBufferSize 0 + a synchronous scheduler make GC deterministic in + // one dispose; the default 10-slot release buffer gets there over a session. + environment = createMockEnvironment({ + store: new Store(new RecordSource(), { + gcReleaseBufferSize: 0, + gcScheduler: run => run(), + }), + }); + + gqlFragment = graphql` + fragment useFragmentMissingDataOwnerRefetchTestUserFragment on User { + birthdate { + day + } + } + `; + // The fragment's owner: what a route or list rendered from. + gqlOwnerQuery = graphql` + query useFragmentMissingDataOwnerRefetchTestOwnerQuery($id: ID!) { + node(id: $id) { + ...useFragmentMissingDataOwnerRefetchTestUserFragment + @dangerously_unaliased_fixme + } + } + `; + // A longer-lived query that overlaps the User record but NOT the birthdate + // record: its retain keeps the User alive across GC, so collecting the + // owner query leaves a dangling `user.birthdate` link. + gqlKeepAliveQuery = graphql` + query useFragmentMissingDataOwnerRefetchTestKeepAliveQuery($id: ID!) { + node(id: $id) { + id + } + } + `; + + ownerOperation = createOperationDescriptor(gqlOwnerQuery, {id: '1'}); + keepAliveOperation = createOperationDescriptor(gqlKeepAliveQuery, {id: '1'}); + environment.commitPayload(ownerOperation, { + node: { + __typename: 'User', + id: '1', + birthdate: {day: 15, month: 7, year: 1991}, + }, + }); + environment.commitPayload(keepAliveOperation, { + node: {__typename: 'User', id: '1'}, + }); + environment.retain(keepAliveOperation); // held for the session + ownerRetention = environment.retain(ownerOperation); + userRef = (environment.lookup(ownerOperation.fragment).data as $FlowFixMe) + .node; +}); + +afterEach(() => { + RelayFeatureFlags.ENABLE_MISSING_DATA_OWNER_REFETCH = false; +}); + +component UserBirthday(userRef: unknown) { + // $FlowFixMe[incompatible-call] + const data = useFragment(gqlFragment, userRef as $FlowFixMe); + return data?.birthdate?.day ?? 'partial'; +} + +component TestHarness( + mode: 'visible' | 'hidden' = 'visible', + userRef: unknown, +) { + return ( + + + + + + + + ); +} + +// Release the owner query and let GC run for real. Root reachability works as +// designed: the keep-alive query keeps the User; the birthdate record, +// reachable only through the owner query, is collected — `user.birthdate` +// now dangles. +function collectOwnerOnlyRecords() { + act(() => { + ownerRetention.dispose(); + }); + const source = environment.getStore().getSource(); + expect(source.get('1')).not.toBe(undefined); + expect(source.get('client:1:birthdate')).toBe(undefined); +} + +// GC bumps no epoch and notifies no subscriber; the dangling link surfaces at +// the next store write that changes the parent record (any mutation or +// revalidation touching the User). +let writeCounter = 0; +function writeOverlappingParentRecord() { + act(() => { + environment.commitUpdate(store => { + store.get('1')?.setValue(`Alice ${++writeCounter}`, 'name'); + }); + }); +} + +function expectSingleForcedOwnerRefetch() { + const operations = environment.mock.getAllOperations(); + expect(operations.length).toBe(1); + expect(operations[0].request.node).toBe(gqlOwnerQuery); + expect(operations[0].request.cacheConfig?.force).toBe(true); + return operations[0]; +} + +async function resolveOwnerRefetch(operation: $FlowFixMe) { + await act(async () => { + environment.mock.resolve(operation, { + data: { + node: { + __typename: 'User', + id: '1', + birthdate: {day: 15, month: 7, year: 1991}, + }, + }, + }); + jest.runAllImmediates(); + }); +} + +test('refetches the owner once and suspends when a subscribed fragment re-reads missing data after GC', async () => { + const renderer = ReactTestingLibrary.render( + , + ); + expect(renderer.container.textContent).toBe('15'); + + collectOwnerOnlyRecords(); + writeOverlappingParentRecord(); + + // Instead of rendering the partial snapshot, the fragment issues one forced + // owner refetch and suspends on it. + expect(renderer.container.textContent).toBe('Fallback'); + const refetch = expectSingleForcedOwnerRefetch(); + + await resolveOwnerRefetch(refetch); + expect(renderer.container.textContent).toBe('15'); +}); + +test('recovers a fragment restored by (hidden → GC → visible)', async () => { + const renderer = ReactTestingLibrary.render( + , + ); + expect(renderer.container.textContent).toBe('15'); + + // Hiding disposes the fragment's store subscription; the write while hidden + // is what the restored hook discovers as missed updates. + renderer.rerender(); + collectOwnerOnlyRecords(); + writeOverlappingParentRecord(); + renderer.rerender(); + act(() => { + jest.runAllImmediates(); + }); + + expect(renderer.container.textContent).toBe('Fallback'); + const refetch = expectSingleForcedOwnerRefetch(); + + await resolveOwnerRefetch(refetch); + expect(renderer.container.textContent).toBe('15'); +}); + +test('recovers a remounted fragment whose ref outlived the owner (fresh mount, missing data)', async () => { + const renderer = ReactTestingLibrary.render( + , + ); + expect(renderer.container.textContent).toBe('15'); + + renderer.unmount(); + collectOwnerOnlyRecords(); + + // A new component mounts with the ref that survived in app state + // (virtualized list row, navigation back-stack…). + const remounted = ReactTestingLibrary.render( + , + ); + + expect(remounted.container.textContent).toBe('Fallback'); + const refetch = expectSingleForcedOwnerRefetch(); + + await resolveOwnerRefetch(refetch); + expect(remounted.container.textContent).toBe('15'); +}); + +test('refetches only once: a transport error does not start a request loop', async () => { + const renderer = ReactTestingLibrary.render( + , + ); + collectOwnerOnlyRecords(); + writeOverlappingParentRecord(); + expect(renderer.container.textContent).toBe('Fallback'); + expect(environment.mock.getAllOperations().length).toBe(1); + + await act(async () => { + environment.mock.reject( + environment.mock.getAllOperations()[0], + new Error('network is down'), + ); + jest.runAllImmediates(); + }); + + // The read is still missing, but the once-per-owner marker holds: no new + // pending operation appears (getAllOperations lists pending ones; the + // rejected refetch is gone), and the fragment falls through to today's + // partial render. + writeOverlappingParentRecord(); + expect(environment.mock.getAllOperations().length).toBe(0); + expect(renderer.container.textContent).toBe('partial'); +}); + +test('the marker clears once data reads back complete: a second GC episode recovers again', async () => { + const renderer = ReactTestingLibrary.render( + , + ); + collectOwnerOnlyRecords(); + writeOverlappingParentRecord(); + await resolveOwnerRefetch(expectSingleForcedOwnerRefetch()); + expect(renderer.container.textContent).toBe('15'); + + // The recovered data is held only by the refetch's temporary retain (the + // owner query's own retain lapsed before episode one). Let that TTL expire — + // GC legitimately collects the birthdate again. + await act(async () => { + jest.runAllTimers(); + }); + expect(environment.getStore().getSource().get('client:1:birthdate')).toBe( + undefined, + ); + + // The data-complete render after episode one cleared the marker, so this + // episode recovers with one more request instead of staying broken. + writeOverlappingParentRecord(); + expect(renderer.container.textContent).toBe('Fallback'); + const refetch = expectSingleForcedOwnerRefetch(); + + await resolveOwnerRefetch(refetch); + expect(renderer.container.textContent).toBe('15'); +}); + +test("a store-wide invalidation ('stale') clears the marker instead of permanently disarming recovery", async () => { + // Episode one fails at transport, so the marker is retained with the data + // still missing. + const renderer = ReactTestingLibrary.render( + , + ); + collectOwnerOnlyRecords(); + writeOverlappingParentRecord(); + await act(async () => { + environment.mock.reject( + environment.mock.getAllOperations()[0], + new Error('network is down'), + ); + jest.runAllImmediates(); + }); + expect(renderer.container.textContent).toBe('partial'); + + // The data heals through an unrelated write, and the whole store is then + // invalidated (e.g. an auth boundary): the complete read now checks as + // 'stale', not 'available'. That must still clear the marker. + await act(async () => { + environment.commitPayload(ownerOperation, { + node: { + __typename: 'User', + id: '1', + birthdate: {day: 15, month: 7, year: 1991}, + }, + }); + environment.commitUpdate(store => { + store.invalidateStore(); + }); + jest.runAllImmediates(); + }); + expect(renderer.container.textContent).toBe('15'); + + // Episode two: expire any temporary retain left by episode one's failed + // refetch, then a retain/release cycle lets GC collect the healed birthdate + // again. Recovery must re-arm — a permanently retained marker would render + // 'partial' here. + await act(async () => { + jest.runAllTimers(); + }); + act(() => { + environment.retain(ownerOperation).dispose(); + }); + expect(environment.getStore().getSource().get('client:1:birthdate')).toBe( + undefined, + ); + writeOverlappingParentRecord(); + + expect(renderer.container.textContent).toBe('Fallback'); + const refetch = expectSingleForcedOwnerRefetch(); + + await resolveOwnerRefetch(refetch); + expect(renderer.container.textContent).toBe('15'); +}); + +test('never re-executes a non-query owner', () => { + const ref = userRef as $FlowFixMe; + const mutationOwnedRef = { + ...ref, + [FRAGMENT_OWNER_KEY]: { + ...ref[FRAGMENT_OWNER_KEY], + node: { + ...ref[FRAGMENT_OWNER_KEY].node, + params: { + ...ref[FRAGMENT_OWNER_KEY].node.params, + operationKind: 'mutation', + }, + }, + }, + }; + collectOwnerOnlyRecords(); + + const renderer = ReactTestingLibrary.render( + , + ); + + // No refetch is attempted for a mutation-owned fragment; the partial render + // is unchanged from today's behavior. + expect(environment.mock.getAllOperations().length).toBe(0); + expect(renderer.container.textContent).toBe('partial'); +}); + +test('does not change behavior when the flag is off', () => { + RelayFeatureFlags.ENABLE_MISSING_DATA_OWNER_REFETCH = false; + + const renderer = ReactTestingLibrary.render( + , + ); + collectOwnerOnlyRecords(); + writeOverlappingParentRecord(); + + // Today's behavior: no request, partial snapshot rendered. + expect(environment.mock.getAllOperations().length).toBe(0); + expect(renderer.container.textContent).toBe('partial'); +}); diff --git a/packages/react-relay/relay-hooks/useFragmentInternal_EXPERIMENTAL.js b/packages/react-relay/relay-hooks/useFragmentInternal_EXPERIMENTAL.js index 6e8519310155e..572892be22bde 100644 --- a/packages/react-relay/relay-hooks/useFragmentInternal_EXPERIMENTAL.js +++ b/packages/react-relay/relay-hooks/useFragmentInternal_EXPERIMENTAL.js @@ -77,6 +77,47 @@ function isMissingData(state: FragmentState): boolean { } } +// Owners already refetched (once) because a fragment read missing data with no +// pending operation — see ENABLE_MISSING_DATA_OWNER_REFETCH below. Keeping a +// marker after a failed recovery (partial response, transport error) is what +// prevents refetch loops; the marker is cleared when the owner query reads +// back without missing data. +const WEAKMAP_SUPPORTED = typeof WeakMap === 'function'; +interface IMap { + get(key: K): V | void; + set(key: K, value: V): IMap; +} +const missingDataRefetchesByEnvironment: IMap< + IEnvironment, + Set, +> = WEAKMAP_SUPPORTED ? new WeakMap() : new Map(); +const MAX_MISSING_DATA_REFETCHES = 1000; +let nextMissingDataRefetchID = 0; + +function markMissingDataRefetch( + environment: IEnvironment, + identifier: string, +): boolean { + let refetches = missingDataRefetchesByEnvironment.get(environment); + if (refetches == null) { + refetches = new Set(); + missingDataRefetchesByEnvironment.set(environment, refetches); + } + if (refetches.has(identifier)) { + return false; + } + if (refetches.size >= MAX_MISSING_DATA_REFETCHES) { + // Bound memory; evicting the oldest marker only re-arms a single refetch + // for that owner. + const oldest = refetches.values().next().value; + if (oldest != null) { + refetches.delete(oldest); + } + } + refetches.add(identifier); + return true; +} + function getMissingClientEdges( state: FragmentState, ): ReadonlyArray | null { @@ -620,6 +661,121 @@ hook useFragmentInternal_EXPERIMENTAL( throw pendingOperationsResult.promise; } } + // A fragment can read missing data with NO pending operation when its + // records were garbage-collected while unobserved: a React + // route was hidden (its store subscription disposed), the owner query's + // retain lapsed, GC correctly collected records only that owner reached, + // and the fragment ref survived in React state. On restore — or on a + // fresh mount with a surviving ref (Activity may either preserve or + // remount the hook, so recovery cannot be limited to committed + // fragments) — the read is partial, and the component would render + // `undefined` for fields the fragment explicitly fetched, crashing + // consumers that trust the schema types. + // + // When enabled, recover by refetching the fragment's owner query once and + // suspending on the request instead of rendering the partial snapshot. + // `force: true` plus a unique QueryResource cache breaker ensure a real + // network request even when a completed QueryResource entry or a + // response-cache layer would otherwise short-circuit it. Only query + // owners are refetched — a mutation or subscription must never + // re-execute. The once-per-owner marker (per environment) prevents + // request loops when the refetch itself cannot fill the data; in that + // case execution falls through to today's partial-render behavior. + // + // Retention of the refetched payload: when the owner query is still + // mounted (the route case) its own retain keeps the data + // durably; when only the fragment ref survived, the data is held by the + // prepare() call's temporary retain (a TEMPORARY_RETAIN_DURATION_MS TTL) + // and is GC-eligible again after it lapses. That is by design — the + // marker clears once the data reads back complete (see the else branch + // below), so a later GC episode simply recovers again with one more + // request rather than staying broken. + // + // For a plural fragment only selectors[0].owner is refetched, matching + // how the rest of this function attributes a plural read to its first + // owner. + if ( + RelayFeatureFlags.ENABLE_MISSING_DATA_OWNER_REFETCH && + fragmentSelector != null + ) { + const fragmentOwner = + fragmentSelector.kind === 'PluralReaderSelector' + ? fragmentSelector.selectors[0].owner + : fragmentSelector.owner; + if (fragmentOwner.node.params.operationKind === 'query') { + const refetchOperation = createOperationDescriptor( + fragmentOwner.node, + fragmentOwner.variables, + {...fragmentOwner.cacheConfig, force: true}, + ); + const activeRequestPromise = getPromiseForActiveRequest( + environment, + refetchOperation.request, + ); + if (activeRequestPromise != null) { + throw activeRequestPromise; + } + // Marking + fetching during render is the same tradeoff QueryResource + // itself makes (it writes its cache during render): a discarded + // concurrent render leaves the marker set with its fetch already in + // flight, which at worst suppresses one later refetch until a + // data-complete render clears the marker. + if (markMissingDataRefetch(environment, fragmentOwner.identifier)) { + const QueryResource = getQueryResourceForEnvironment(environment); + const cacheBreaker = `missing-data-${nextMissingDataRefetchID++}`; + QueryResource.prepare( + refetchOperation, + fetchQueryInternal(environment, refetchOperation), + 'network-only', + undefined, + undefined, + cacheBreaker, + undefined, + ); + // Defensive parity with the activeRequestPromise check above: with + // an async network, prepare() already throws the pending promise + // itself, so this only fires if prepare() returned synchronously + // (e.g. a synchronous network resolved the payload). + const refetchPromise = getPromiseForActiveRequest( + environment, + refetchOperation.request, + ); + if (refetchPromise != null) { + throw refetchPromise; + } + } + } + } + } else if ( + RelayFeatureFlags.ENABLE_MISSING_DATA_OWNER_REFETCH && + fragmentSelector != null + ) { + const fragmentOwner = + fragmentSelector.kind === 'PluralReaderSelector' + ? fragmentSelector.selectors[0].owner + : fragmentSelector.owner; + const refetches = missingDataRefetchesByEnvironment.get(environment); + if ( + refetches != null && + refetches.has(fragmentOwner.identifier) && + fragmentOwner.node.params.operationKind === 'query' + ) { + // Clear the marker only once the whole owner query reads back without + // missing data: a sibling fragment of a partially-failed recovery must + // not re-arm the refetch (clearing while 'missing' would loop forever + // against a server that keeps returning partial data). 'stale' — data + // fully present but invalidated, e.g. invalidateStore() at an auth + // boundary — is loop-safe and must clear, or the owner would stay + // permanently exempt from recovery in later GC episodes. + const ownerOperation = createOperationDescriptor( + fragmentOwner.node, + fragmentOwner.variables, + fragmentOwner.cacheConfig, + ); + if (environment.check(ownerOperation).status !== 'missing') { + refetches.delete(fragmentOwner.identifier); + } + } } // Report required fields only if we're not suspending, since that means diff --git a/packages/relay-runtime/util/RelayFeatureFlags.js b/packages/relay-runtime/util/RelayFeatureFlags.js index 26e3c444923cf..bf84340a3622e 100644 --- a/packages/relay-runtime/util/RelayFeatureFlags.js +++ b/packages/relay-runtime/util/RelayFeatureFlags.js @@ -56,6 +56,12 @@ export type FeatureFlags = { // Temporary flag to experiment to enable compatibility with React's unstable API ENABLE_ACTIVITY_COMPATIBILITY: boolean, + // When a fragment reads missing data and no operation affecting its owner is + // pending (e.g. its records were GC'd while a React was hidden + // and the fragment ref outlived the owner query's retain), refetch the owner + // query once and suspend, instead of rendering a partial snapshot. + ENABLE_MISSING_DATA_OWNER_REFETCH: boolean, + // Adds a prefix to the storage key of read time resolvers. This is used to // disambiguate the same resolver being used at both read time and exec time. ENABLE_READ_TIME_RESOLVER_STORAGE_KEY_PREFIX: boolean, @@ -138,6 +144,7 @@ const RelayFeatureFlags: FeatureFlags = { MARK_RESOLVER_VALUES_AS_CLEAN_AFTER_FRAGMENT_REREAD: false, ENABLE_CYLE_DETECTION_IN_VARIABLES: false, ENABLE_ACTIVITY_COMPATIBILITY: true, + ENABLE_MISSING_DATA_OWNER_REFETCH: false, ENABLE_READ_TIME_RESOLVER_STORAGE_KEY_PREFIX: true, ENABLE_USE_PAGINATION_IS_LOADING_FIX: false, DISALLOW_NESTED_UPDATES: false,