Skip to content

Commit a54e181

Browse files
captbaritonefacebook-github-bot
captbaritone
authored and
facebook-github-bot
committed
Fix for weak resolver GC bug
Reviewed By: tyao1 Differential Revision: D79911907 fbshipit-source-id: b02bc7beaddc40bc008a34609c5c083ccbd60ed4
1 parent 00863b9 commit a54e181

5 files changed

Lines changed: 21 additions & 116 deletions

File tree

‎packages/react-relay/__tests__/LiveResolvers-test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1507,7 +1507,7 @@ describe('client-only fragments', () => {
15071507
expect(() => {
15081508
GLOBAL_STORE.dispatch({type: 'INCREMENT'});
15091509
}).toThrowError(
1510-
'Unexpected LiveState value returned from Relay Resolver internal field `RELAY_RESOLVER_LIVE_STATE_VALUE`. It is likely a bug in Relay, or a corrupt state of the relay store state Field Path `counter_suspends_when_odd`. Record `{"__id":"client:1:$r:counter_suspends_when_odd","__typename":"__RELAY_RESOLVER__","__resolverError":null,"__resolverValue":{"__LIVE_RESOLVER_SUSPENSE_SENTINEL":true},"__resolverLiveStateDirty":true}`.',
1510+
'Unexpected LiveState value returned from Relay Resolver internal field `RELAY_RESOLVER_LIVE_STATE_VALUE`. It is likely a bug in Relay, or a corrupt state of the relay store state Field Path `counter_suspends_when_odd`. Record `{\"__id\":\"client:1:$r:counter_suspends_when_odd\",\"__typename\":\"__RELAY_RESOLVER__\",\"__resolverError\":null,\"__resolverValue\":{\"__LIVE_RESOLVER_SUSPENSE_SENTINEL\":true},\"__resolverOutputTypeRecordIDs\":{},\"__resolverLiveStateDirty\":true}`.',
15111511
);
15121512
// $FlowFixMe[incompatible-use]
15131513
expect(renderer.toJSON()).toEqual('Loading...');

‎packages/react-relay/__tests__/RelayResolvers-withOutputType-test.js‎

Lines changed: 0 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ const {
3333
const RelayModernEnvironment = require('relay-runtime/store/RelayModernEnvironment');
3434
const RelayModernStore = require('relay-runtime/store/RelayModernStore.js');
3535
const RelayRecordSource = require('relay-runtime/store/RelayRecordSource');
36-
const {
37-
RELAY_READ_TIME_RESOLVER_KEY_PREFIX,
38-
} = require('relay-runtime/store/RelayStoreUtils');
3936
const {
4037
disallowConsoleErrors,
4138
disallowWarnings,
@@ -561,91 +558,6 @@ test('renders after GC', () => {
561558
(environment.getStore(): $FlowFixMe).__gc();
562559
jest.runAllTimers();
563560

564-
expect(environment.getStore().getSource().toJSON()).toEqual({
565-
'client:root': {
566-
__id: 'client:root',
567-
__typename: '__Root',
568-
// $FlowFixMe[invalid-computed-prop]
569-
[`${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`]: {
570-
__ref: `client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`,
571-
},
572-
},
573-
// $FlowFixMe[invalid-computed-prop]
574-
[`client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`]: {
575-
__id: `client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`,
576-
__resolverError: null,
577-
__resolverLiveStateDirty: false,
578-
__resolverLiveStateSubscription: expect.anything(),
579-
__resolverLiveStateValue: {
580-
read: expect.anything(),
581-
subscribe: expect.anything(),
582-
},
583-
__resolverOutputTypeRecordIDs: new Set([
584-
`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`,
585-
`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0`,
586-
`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node`,
587-
`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):pageInfo`,
588-
]),
589-
__resolverSnapshot: undefined,
590-
__resolverValue: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`,
591-
__typename: '__RELAY_RESOLVER__',
592-
},
593-
// $FlowFixMe[invalid-computed-prop]
594-
[`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`]:
595-
{
596-
__id: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10)`,
597-
__typename: 'TodoConnection',
598-
count: 1,
599-
edges: {
600-
__refs: [
601-
`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0`,
602-
],
603-
},
604-
pageInfo: {
605-
__ref: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):pageInfo`,
606-
},
607-
},
608-
// $FlowFixMe[invalid-computed-prop]
609-
[`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0`]:
610-
{
611-
__id: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0`,
612-
__typename: 'TodoEdge',
613-
cursor: null,
614-
node: {
615-
__ref: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node`,
616-
},
617-
},
618-
// $FlowFixMe[invalid-computed-prop]
619-
[`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node`]:
620-
{
621-
__id: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node`,
622-
__typename: 'Todo',
623-
// $FlowFixMe[invalid-computed-prop]
624-
[`${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}complete`]: {
625-
__ref: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}complete`,
626-
},
627-
// $FlowFixMe[invalid-computed-prop]
628-
[`${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}self`]: {
629-
__ref: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}self`,
630-
},
631-
// $FlowFixMe[invalid-computed-prop]
632-
[`${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}text`]: {
633-
__ref: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):edges:0:node:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}text`,
634-
},
635-
todo_id: 'todo-1',
636-
},
637-
// $FlowFixMe[invalid-computed-prop]
638-
[`client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):pageInfo`]:
639-
{
640-
__id: `client:TodoConnection:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}todos(first:10):pageInfo`,
641-
__typename: 'TodoConnectionPageInfo',
642-
endCursor: null,
643-
hasNextPage: false,
644-
hasPreviousPage: false,
645-
startCursor: null,
646-
},
647-
});
648-
649561
expect(() => {
650562
TestRenderer.act(() => {
651563
renderer.update(

‎packages/relay-runtime/store/RelayReferenceMarker.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,16 +255,17 @@ class RelayReferenceMarker {
255255
if (resolverRecord == null) {
256256
return;
257257
}
258+
const {linkedField} = field;
258259
if (field.backingField.isOutputType) {
259260
// Mark all @outputType record IDs
260261
const outputTypeRecordIDs = getOutputTypeRecordIDs(resolverRecord);
261262
if (outputTypeRecordIDs != null) {
262263
for (const dataID of outputTypeRecordIDs) {
263264
this._references.add(dataID);
265+
this._traverse(linkedField, dataID);
264266
}
265267
}
266268
} else {
267-
const {linkedField} = field;
268269
const concreteType = linkedField.concreteType;
269270
if (concreteType == null) {
270271
// TODO: Handle retaining abstract client edges to client types.

‎packages/relay-runtime/store/__tests__/resolvers/ResolverGC-test.js‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,6 @@ test('Resolver reading a client-edge to a client type (recursive)', async () =>
828828
});
829829

830830
test('Resolver reading a weak edge', async () => {
831-
let calls = 0;
832831
await testResolverGC({
833832
query: graphql`
834833
query ResolverGCTestWeakQuery {
@@ -862,29 +861,12 @@ test('Resolver reading a weak edge', async () => {
862861
some_todo_description: {text: 'some todo description'},
863862
});
864863

865-
// TODO: Delete this case once the bug is fixed
866-
if (calls === 0) {
867-
// This function gets called twice. Once after GC and again after a second
868-
// lookup call. After GC this record is missing but after a second lookup it's returned.
869-
expect(recordIdsInStore).toEqual([
870-
'client:root',
871-
`client:TodoDescription:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description`,
872-
`client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description`,
873-
874-
// `client:TodoDescription:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description:$r:text`,
875-
]);
876-
} else if (calls === 1) {
877-
expect(recordIdsInStore).toEqual([
878-
'client:root',
879-
`client:TodoDescription:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description`,
880-
`client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description`,
881-
`client:TodoDescription:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description:$r:text`,
882-
]);
883-
} else {
884-
throw new Error('More calls than expected');
885-
}
886-
887-
calls++;
864+
expect(recordIdsInStore).toEqual([
865+
'client:root',
866+
`client:TodoDescription:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description`,
867+
`client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description`,
868+
`client:TodoDescription:client:root:${RELAY_READ_TIME_RESOLVER_KEY_PREFIX}some_todo_description:$r:text`,
869+
]);
888870
},
889871
afterFreedGC: recordIdsInStore => {
890872
expect(recordIdsInStore).toEqual(['client:root']);

‎packages/relay-runtime/store/live-resolvers/LiveResolverCache.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -623,13 +623,23 @@ class LiveResolverCache implements ResolverCache {
623623
);
624624
} else {
625625
shallowFreeze(value);
626-
// For "classic" resolvers (or if the value is nullish), we are just setting their
627-
// value as is.
626+
627+
// For "classic" resolvers (or if the value is nullish/suspense), we are
628+
// set their value...
628629
RelayModernRecord.setValue(
629630
resolverRecord,
630631
RELAY_RESOLVER_VALUE_KEY,
631632
value,
632633
);
634+
635+
// ...and clear the output type record IDs, if any since a resolver must
636+
// be an output type resolver with a non-suspended, non-null value to
637+
// refernce any output type record ids.
638+
RelayModernRecord.setValue(
639+
resolverRecord,
640+
RELAY_RESOLVER_OUTPUT_TYPE_RECORD_IDS,
641+
new Set(),
642+
);
633643
}
634644

635645
return updatedDataIDs;

0 commit comments

Comments
 (0)