From ed612b84370da621ed337f2def7a6a2a60396554 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Wed, 5 Apr 2023 14:02:45 -0700 Subject: [PATCH] Add feature flag for operation tracking that would work with lazy React notification Reviewed By: josephsavona Differential Revision: D43716322 fbshipit-source-id: ceb97e895951d64539e97e5fbfdbc8ccf59010dc --- ...gmentResource-WithOperationTracker-test.js | 756 ++++++------ .../store/RelayStoreSubscriptions.js | 15 + ...rnEnvironment-WithOperationTracker-test.js | 1036 +++++++++-------- ...thOperationTrackerTest1Mutation.graphql.js | 19 +- .../store/hasSignificantOverlappingIDs.js | 38 + .../relay-runtime/util/RelayFeatureFlags.js | 10 + 6 files changed, 1042 insertions(+), 832 deletions(-) create mode 100644 packages/relay-runtime/store/hasSignificantOverlappingIDs.js diff --git a/packages/react-relay/relay-hooks/__tests__/FragmentResource-WithOperationTracker-test.js b/packages/react-relay/relay-hooks/__tests__/FragmentResource-WithOperationTracker-test.js index 7e3c02bbba0c9..f41cc56cb6ace 100644 --- a/packages/react-relay/relay-hooks/__tests__/FragmentResource-WithOperationTracker-test.js +++ b/packages/react-relay/relay-hooks/__tests__/FragmentResource-WithOperationTracker-test.js @@ -24,435 +24,447 @@ const { graphql, } = require('relay-runtime'); const RelayOperationTracker = require('relay-runtime/store/RelayOperationTracker'); +const RelayFeatureFlags = require('relay-runtime/util/RelayFeatureFlags'); const {createMockEnvironment} = require('relay-test-utils'); const {disallowWarnings} = require('relay-test-utils-internal'); disallowWarnings(); -describe('FragmentResource with Operation Tracker and Missing Data', () => { - const componentName = 'TestComponent'; - let environment; - let NodeQuery; - let ViewerFriendsQuery; - let FriendsPaginationQuery; - let UserFragment; - let PlainUserNameRenderer_name; - let PlainUserNameRenderer_name$normalization; - let FragmentResource; - let operationLoader; - let operationTracker; - let viewerOperation; - let nodeOperation; - let logger; +describe.each([true, false])( + 'FragmentResource with Operation Tracker and Missing Data with ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION=%p', + looseAttribution => { + const componentName = 'TestComponent'; + let environment; + let NodeQuery; + let ViewerFriendsQuery; + let FriendsPaginationQuery; + let UserFragment; + let PlainUserNameRenderer_name; + let PlainUserNameRenderer_name$normalization; + let FragmentResource; + let operationLoader; + let operationTracker; + let viewerOperation; + let nodeOperation; + let logger; - beforeEach(() => { - operationLoader = { - load: jest.fn<[mixed], Promise>(), - get: jest.fn<[mixed], ?NormalizationRootNode>(), - }; - operationTracker = new RelayOperationTracker(); - logger = jest.fn<[LogEvent], void>(); - environment = createMockEnvironment({ - operationTracker, - operationLoader, - log: logger, - }); - NodeQuery = graphql` - query FragmentResourceWithOperationTrackerTestNodeQuery($id: ID!) { - node(id: $id) { - ...FragmentResourceWithOperationTrackerTestUserFragment + beforeEach(() => { + RelayFeatureFlags.ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION = + looseAttribution; + operationLoader = { + load: jest.fn<[mixed], Promise>(), + get: jest.fn<[mixed], ?NormalizationRootNode>(), + }; + operationTracker = new RelayOperationTracker(); + logger = jest.fn<[LogEvent], void>(); + environment = createMockEnvironment({ + operationTracker, + operationLoader, + log: logger, + }); + NodeQuery = graphql` + query FragmentResourceWithOperationTrackerTestNodeQuery($id: ID!) { + node(id: $id) { + ...FragmentResourceWithOperationTrackerTestUserFragment + } } - } - `; - ViewerFriendsQuery = graphql` - query FragmentResourceWithOperationTrackerTestViewerFriendsQuery { - viewer { - actor { - friends(first: 1) @connection(key: "Viewer_friends") { - edges { - node { - ...FragmentResourceWithOperationTrackerTestUserFragment + `; + ViewerFriendsQuery = graphql` + query FragmentResourceWithOperationTrackerTestViewerFriendsQuery { + viewer { + actor { + friends(first: 1) @connection(key: "Viewer_friends") { + edges { + node { + ...FragmentResourceWithOperationTrackerTestUserFragment + } } } } } } - } - `; - FriendsPaginationQuery = graphql` - query FragmentResourceWithOperationTrackerTestFriendsPaginationQuery( - $id: ID! - ) { - node(id: $id) { - ... on User { - friends(first: 1) @connection(key: "Viewer_friends") { - edges { - node { - ...FragmentResourceWithOperationTrackerTestUserFragment + `; + FriendsPaginationQuery = graphql` + query FragmentResourceWithOperationTrackerTestFriendsPaginationQuery( + $id: ID! + ) { + node(id: $id) { + ... on User { + friends(first: 1) @connection(key: "Viewer_friends") { + edges { + node { + ...FragmentResourceWithOperationTrackerTestUserFragment + } } } } } } - } - `; - PlainUserNameRenderer_name = graphql` - fragment FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name on PlainUserNameRenderer { - plaintext - data { - text - } - } - `; - graphql` - fragment FragmentResourceWithOperationTrackerTestMarkdownUserNameRenderer_name on MarkdownUserNameRenderer { - markdown - data { - markup + `; + PlainUserNameRenderer_name = graphql` + fragment FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name on PlainUserNameRenderer { + plaintext + data { + text + } } - } - `; - PlainUserNameRenderer_name$normalization = require('./__generated__/FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name$normalization.graphql'); - UserFragment = graphql` - fragment FragmentResourceWithOperationTrackerTestUserFragment on User { - id - name - nameRenderer @match { - ...FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name - @module(name: "PlainUserNameRenderer.react") - ...FragmentResourceWithOperationTrackerTestMarkdownUserNameRenderer_name - @module(name: "MarkdownUserNameRenderer.react") + `; + graphql` + fragment FragmentResourceWithOperationTrackerTestMarkdownUserNameRenderer_name on MarkdownUserNameRenderer { + markdown + data { + markup + } } - plainNameRenderer: nameRenderer - @match( - key: "FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer" - ) { - ...FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name - @module(name: "PlainUserNameRenderer.react") + `; + PlainUserNameRenderer_name$normalization = require('./__generated__/FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name$normalization.graphql'); + UserFragment = graphql` + fragment FragmentResourceWithOperationTrackerTestUserFragment on User { + id + name + nameRenderer @match { + ...FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name + @module(name: "PlainUserNameRenderer.react") + ...FragmentResourceWithOperationTrackerTestMarkdownUserNameRenderer_name + @module(name: "MarkdownUserNameRenderer.react") + } + plainNameRenderer: nameRenderer + @match( + key: "FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer" + ) { + ...FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name + @module(name: "PlainUserNameRenderer.react") + } } - } - `; + `; - FragmentResource = createFragmentResource(environment); - viewerOperation = createOperationDescriptor(ViewerFriendsQuery, {}); - nodeOperation = createOperationDescriptor(NodeQuery, { - id: 'user-id-1', - }); - environment.execute({operation: viewerOperation}).subscribe({}); - environment.subscribe( - environment.lookup(viewerOperation.fragment), - jest.fn(), - ); + FragmentResource = createFragmentResource(environment); + viewerOperation = createOperationDescriptor(ViewerFriendsQuery, {}); + nodeOperation = createOperationDescriptor(NodeQuery, { + id: 'user-id-1', + }); + environment.execute({operation: viewerOperation}).subscribe({}); + environment.subscribe( + environment.lookup(viewerOperation.fragment), + jest.fn(), + ); - environment.mock.resolve(viewerOperation, { - data: { - viewer: { - actor: { - id: 'viewer-id', - __typename: 'User', - friends: { - pageInfo: { - hasNextPage: true, - hasPrevPage: false, - startCursor: 'cursor-1', - endCursor: 'cursor-1', - }, - edges: [ - { - cursor: 'cursor-1', - node: { - id: 'user-id-1', - name: 'Alice', - __typename: 'User', - nameRenderer: null, - plainNameRenderer: null, - }, + environment.mock.resolve(viewerOperation, { + data: { + viewer: { + actor: { + id: 'viewer-id', + __typename: 'User', + friends: { + pageInfo: { + hasNextPage: true, + hasPrevPage: false, + startCursor: 'cursor-1', + endCursor: 'cursor-1', }, - ], + edges: [ + { + cursor: 'cursor-1', + node: { + id: 'user-id-1', + name: 'Alice', + __typename: 'User', + nameRenderer: null, + plainNameRenderer: null, + }, + }, + ], + }, }, }, }, - }, - }); + }); - // We need to subscribe to a fragment in order for OperationTracker - // to be able to notify owners if they are affected by any pending operation - environment.subscribe( - environment.lookup( - createReaderSelector( - UserFragment, - 'user-id-1', - viewerOperation.request.variables, - viewerOperation.request, + // We need to subscribe to a fragment in order for OperationTracker + // to be able to notify owners if they are affected by any pending operation + environment.subscribe( + environment.lookup( + createReaderSelector( + UserFragment, + 'user-id-1', + viewerOperation.request.variables, + viewerOperation.request, + ), ), - ), - jest.fn(), - ); - }); + jest.fn(), + ); + }); + + afterEach(() => { + RelayFeatureFlags.ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION = false; + }); - it('should throw and cache promise for pending operation affecting fragment owner', () => { - environment.execute({operation: nodeOperation}).subscribe({}); - operationLoader.load.mockImplementation(() => - Promise.resolve(PlainUserNameRenderer_name$normalization), - ); - environment.mock.nextValue(nodeOperation, { - data: { - node: { - __typename: 'User', - id: 'user-id-1', - name: 'Alice', - nameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_FragmentResourceWithOperationTrackerTestUserFragment: - 'PlainUserNameRenderer.react', - __module_operation_FragmentResourceWithOperationTrackerTestUserFragment: - 'FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name$normalization.graphql', - plaintext: 'Plaintext', - data: { - id: 'plain-test-data-id-1', - text: 'Data Text', + it('should throw and cache promise for pending operation affecting fragment owner', () => { + environment.execute({operation: nodeOperation}).subscribe({}); + operationLoader.load.mockImplementation(() => + Promise.resolve(PlainUserNameRenderer_name$normalization), + ); + environment.mock.nextValue(nodeOperation, { + data: { + node: { + __typename: 'User', + id: 'user-id-1', + name: 'Alice', + nameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_FragmentResourceWithOperationTrackerTestUserFragment: + 'PlainUserNameRenderer.react', + __module_operation_FragmentResourceWithOperationTrackerTestUserFragment: + 'FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name$normalization.graphql', + plaintext: 'Plaintext', + data: { + id: 'plain-test-data-id-1', + text: 'Data Text', + }, }, - }, - plainNameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: - 'PlainUserNameRenderer.react', - __module_operation_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: - 'FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name$normalization.graphql', - plaintext: 'Plaintext', - data: { - id: 'plain-test-data-id-1', - text: 'Data Text', + plainNameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: + 'PlainUserNameRenderer.react', + __module_operation_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: + 'FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name$normalization.graphql', + plaintext: 'Plaintext', + data: { + id: 'plain-test-data-id-1', + text: 'Data Text', + }, }, }, }, - }, - }); - expect(operationLoader.load).toBeCalledTimes(2); + }); + expect(operationLoader.load).toBeCalledTimes(2); - // Calling `complete` here will just mark network request as completed, but - // we still need to process follow-ups with normalization ASTs by resolving - // the operation loader promise - environment.mock.complete(nodeOperation); + // Calling `complete` here will just mark network request as completed, but + // we still need to process follow-ups with normalization ASTs by resolving + // the operation loader promise + environment.mock.complete(nodeOperation); - const fragmentRef = { - __id: 'client:user-id-1:nameRenderer(supported:["PlainUserNameRenderer"])', - __fragments: { - FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name: {}, - }, - __fragmentOwner: viewerOperation.request, - }; + const fragmentRef = { + __id: 'client:user-id-1:nameRenderer(supported:["PlainUserNameRenderer"])', + __fragments: { + FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name: + {}, + }, + __fragmentOwner: viewerOperation.request, + }; - let thrown = null; - try { - FragmentResource.read( - PlainUserNameRenderer_name, - fragmentRef, - componentName, - ); - } catch (promise) { - expect(promise).toBeInstanceOf(Promise); - thrown = promise; - } - expect(thrown).not.toBe(null); + let thrown = null; + try { + FragmentResource.read( + PlainUserNameRenderer_name, + fragmentRef, + componentName, + ); + } catch (promise) { + expect(promise).toBeInstanceOf(Promise); + thrown = promise; + } + expect(thrown).not.toBe(null); - // Try reading fragment a second time while affecting operation is pending - let cached = null; - try { - FragmentResource.read( - PlainUserNameRenderer_name, - fragmentRef, - componentName, - ); - } catch (promise) { - expect(promise).toBeInstanceOf(Promise); - cached = promise; - } - // Assert that promise from first read was cached - expect(cached).toBe(thrown); + // Try reading fragment a second time while affecting operation is pending + let cached = null; + try { + FragmentResource.read( + PlainUserNameRenderer_name, + fragmentRef, + componentName, + ); + } catch (promise) { + expect(promise).toBeInstanceOf(Promise); + cached = promise; + } + // Assert that promise from first read was cached + expect(cached).toBe(thrown); - // Assert that we logged a 'pendingoperation.found' event. - const pendingOperationFoundEvents = logger.mock.calls - .map(([event]) => event) - .filter(event => event.name === 'pendingoperation.found'); + // Assert that we logged a 'pendingoperation.found' event. + const pendingOperationFoundEvents = logger.mock.calls + .map(([event]) => event) + .filter(event => event.name === 'pendingoperation.found'); - expect(pendingOperationFoundEvents.length).toBe(1); - const event = pendingOperationFoundEvents[0]; - invariant( - event.name === 'pendingoperation.found', - "Expected log event to be 'pendingoperation.found'", - ); - expect(event.fragment.name).toBe( - 'FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name', - ); - expect(event.fragmentOwner.node.operation.name).toBe( - viewerOperation.request.node.operation.name, - ); - expect( - event.pendingOperations.map(owner => owner.node.operation.name), - ).toEqual(['FragmentResourceWithOperationTrackerTestNodeQuery']); - }); + expect(pendingOperationFoundEvents.length).toBe(1); + const event = pendingOperationFoundEvents[0]; + invariant( + event.name === 'pendingoperation.found', + "Expected log event to be 'pendingoperation.found'", + ); + expect(event.fragment.name).toBe( + 'FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name', + ); + expect(event.fragmentOwner.node.operation.name).toBe( + viewerOperation.request.node.operation.name, + ); + expect( + event.pendingOperations.map(owner => owner.node.operation.name), + ).toEqual(['FragmentResourceWithOperationTrackerTestNodeQuery']); + }); - it('should read the data from the store once operation fully completed', () => { - environment.execute({operation: nodeOperation}).subscribe({}); - operationLoader.load.mockImplementation(() => - Promise.resolve(PlainUserNameRenderer_name$normalization), - ); - environment.mock.nextValue(nodeOperation, { - data: { - node: { - __typename: 'User', - id: 'user-id-1', - name: 'Alice', - nameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_FragmentResourceWithOperationTrackerTestUserFragment: - 'PlainUserNameRenderer.react', - __module_operation_FragmentResourceWithOperationTrackerTestUserFragment: - 'PlainUserNameRenderer_name$normalization.graphql', - plaintext: 'Plaintext', - data: { - id: 'plain-test-data-id-1', - text: 'Data Text', + it('should read the data from the store once operation fully completed', () => { + environment.execute({operation: nodeOperation}).subscribe({}); + operationLoader.load.mockImplementation(() => + Promise.resolve(PlainUserNameRenderer_name$normalization), + ); + environment.mock.nextValue(nodeOperation, { + data: { + node: { + __typename: 'User', + id: 'user-id-1', + name: 'Alice', + nameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_FragmentResourceWithOperationTrackerTestUserFragment: + 'PlainUserNameRenderer.react', + __module_operation_FragmentResourceWithOperationTrackerTestUserFragment: + 'PlainUserNameRenderer_name$normalization.graphql', + plaintext: 'Plaintext', + data: { + id: 'plain-test-data-id-1', + text: 'Data Text', + }, }, - }, - plainNameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: - 'PlainUserNameRenderer.react', - __module_operation_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: - 'PlainUserNameRenderer_name$normalization.graphql', - plaintext: 'Plaintext', - data: { - id: 'plain-test-data-id-1', - text: 'Data Text', + plainNameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: + 'PlainUserNameRenderer.react', + __module_operation_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: + 'PlainUserNameRenderer_name$normalization.graphql', + plaintext: 'Plaintext', + data: { + id: 'plain-test-data-id-1', + text: 'Data Text', + }, }, }, }, - }, - }); - expect(operationLoader.load).toBeCalledTimes(2); - environment.mock.complete(nodeOperation); - // To make sure promise is resolved - jest.runAllTimers(); - const snapshot = FragmentResource.read( - PlainUserNameRenderer_name, - { - __id: 'client:user-id-1:nameRenderer(supported:["PlainUserNameRenderer"])', - __fragments: { - FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name: - {}, + }); + expect(operationLoader.load).toBeCalledTimes(2); + environment.mock.complete(nodeOperation); + // To make sure promise is resolved + jest.runAllTimers(); + const snapshot = FragmentResource.read( + PlainUserNameRenderer_name, + { + __id: 'client:user-id-1:nameRenderer(supported:["PlainUserNameRenderer"])', + __fragments: { + FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name: + {}, + }, + __fragmentOwner: viewerOperation.request, }, - __fragmentOwner: viewerOperation.request, - }, - componentName, - ); - expect(snapshot.data).toEqual({ - data: { - text: 'Data Text', - }, - plaintext: 'Plaintext', + componentName, + ); + expect(snapshot.data).toEqual({ + data: { + text: 'Data Text', + }, + plaintext: 'Plaintext', + }); }); - }); - it('should suspend on pagination query and then read the data', () => { - const paginationOperation = createOperationDescriptor( - FriendsPaginationQuery, - { - id: 'viewer-id', - }, - ); - environment.execute({operation: paginationOperation}).subscribe({}); - operationLoader.load.mockImplementation(() => - Promise.resolve(PlainUserNameRenderer_name$normalization), - ); - environment.mock.nextValue(paginationOperation, { - data: { - node: { - __typename: 'User', + it('should suspend on pagination query and then read the data', () => { + const paginationOperation = createOperationDescriptor( + FriendsPaginationQuery, + { id: 'viewer-id', - friends: { - pageInfo: { - hasNextPage: true, - hasPrevPage: false, - startCursor: 'cursor-2', - endCursor: 'cursor-2', - }, - edges: [ - { - cursor: 'cursor-2', - node: { - __typename: 'User', - id: 'user-id-2', - name: 'Bob', - nameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_FragmentResourceWithOperationTrackerTestUserFragment: - 'PlainUserNameRenderer.react', - __module_operation_FragmentResourceWithOperationTrackerTestUserFragment: - 'PlainUserNameRenderer_name$normalization.graphql', - plaintext: 'Plaintext 2', - data: { - id: 'plain-test-data-id-2', + }, + ); + environment.execute({operation: paginationOperation}).subscribe({}); + operationLoader.load.mockImplementation(() => + Promise.resolve(PlainUserNameRenderer_name$normalization), + ); + environment.mock.nextValue(paginationOperation, { + data: { + node: { + __typename: 'User', + id: 'viewer-id', + friends: { + pageInfo: { + hasNextPage: true, + hasPrevPage: false, + startCursor: 'cursor-2', + endCursor: 'cursor-2', + }, + edges: [ + { + cursor: 'cursor-2', + node: { + __typename: 'User', + id: 'user-id-2', + name: 'Bob', + nameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_FragmentResourceWithOperationTrackerTestUserFragment: + 'PlainUserNameRenderer.react', + __module_operation_FragmentResourceWithOperationTrackerTestUserFragment: + 'PlainUserNameRenderer_name$normalization.graphql', + plaintext: 'Plaintext 2', + data: { + id: 'plain-test-data-id-2', - text: 'Data Text 2', + text: 'Data Text 2', + }, }, - }, - plainNameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: - 'PlainUserNameRenderer.react', - __module_operation_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: - 'PlainUserNameRenderer_name$normalization.graphql', - plaintext: 'Plaintext 2', - data: { - id: 'plain-test-data-id-2', - text: 'Data Text 2', + plainNameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: + 'PlainUserNameRenderer.react', + __module_operation_FragmentResourceWithOperationTrackerTestUserFragment_plainNameRenderer: + 'PlainUserNameRenderer_name$normalization.graphql', + plaintext: 'Plaintext 2', + data: { + id: 'plain-test-data-id-2', + text: 'Data Text 2', + }, }, }, }, - }, - ], + ], + }, }, }, - }, - }); - expect(operationLoader.load).toBeCalledTimes(2); - const fragmentRef = { - __id: 'client:user-id-2:nameRenderer(supported:["PlainUserNameRenderer"])', - __fragments: { - FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name: {}, - }, - __fragmentOwner: viewerOperation.request, - }; - let promiseThrown = false; - try { - FragmentResource.read( + }); + expect(operationLoader.load).toBeCalledTimes(2); + const fragmentRef = { + __id: 'client:user-id-2:nameRenderer(supported:["PlainUserNameRenderer"])', + __fragments: { + FragmentResourceWithOperationTrackerTestPlainUserNameRenderer_name: + {}, + }, + __fragmentOwner: viewerOperation.request, + }; + let promiseThrown = false; + try { + FragmentResource.read( + PlainUserNameRenderer_name, + fragmentRef, + componentName, + ); + } catch (promise) { + expect(promise).toBeInstanceOf(Promise); + promiseThrown = true; + } + expect(promiseThrown).toBe(true); + + // Complete the request + environment.mock.complete(paginationOperation); + // This should resolve promises + jest.runAllTimers(); + + const snapshot = FragmentResource.read( PlainUserNameRenderer_name, fragmentRef, componentName, ); - } catch (promise) { - expect(promise).toBeInstanceOf(Promise); - promiseThrown = true; - } - expect(promiseThrown).toBe(true); - - // Complete the request - environment.mock.complete(paginationOperation); - // This should resolve promises - jest.runAllTimers(); - - const snapshot = FragmentResource.read( - PlainUserNameRenderer_name, - fragmentRef, - componentName, - ); - expect(snapshot.data).toEqual({ - data: { - text: 'Data Text 2', - }, - plaintext: 'Plaintext 2', + expect(snapshot.data).toEqual({ + data: { + text: 'Data Text 2', + }, + plaintext: 'Plaintext 2', + }); }); - }); -}); + }, +); diff --git a/packages/relay-runtime/store/RelayStoreSubscriptions.js b/packages/relay-runtime/store/RelayStoreSubscriptions.js index 413a492e5f645..10470dfd8cf73 100644 --- a/packages/relay-runtime/store/RelayStoreSubscriptions.js +++ b/packages/relay-runtime/store/RelayStoreSubscriptions.js @@ -27,6 +27,7 @@ const deepFreeze = require('../util/deepFreeze'); const recycleNodesInto = require('../util/recycleNodesInto'); const RelayFeatureFlags = require('../util/RelayFeatureFlags'); const hasOverlappingIDs = require('./hasOverlappingIDs'); +const hasSignificantOverlappingIDs = require('./hasSignificantOverlappingIDs'); const RelayReader = require('./RelayReader'); type Subscription = { @@ -95,6 +96,8 @@ class RelayStoreSubscriptions implements StoreSubscriptions { subscription.backup = null; if (backup) { if (backup.data !== subscription.snapshot.data) { + // This subscription's data changed in the optimistic state. We will + // need to re-read. subscription.stale = true; } subscription.snapshot = { @@ -108,6 +111,8 @@ class RelayStoreSubscriptions implements StoreSubscriptions { relayResolverErrors: backup.relayResolverErrors, }; } else { + // This subscription was created during the optimisitic state. We should + // re-read. subscription.stale = true; } }); @@ -188,6 +193,16 @@ class RelayStoreSubscriptions implements StoreSubscriptions { callback(nextSnapshot); return snapshot.selector.owner; } + // While there were some overlapping IDs that affected this subscription, + // none of the read fields were actually affected. + if ( + RelayFeatureFlags.ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION && + (stale || + hasSignificantOverlappingIDs(snapshot.seenRecords, updatedRecordIDs)) + ) { + // With loose attribution enabled, we'll attribute this anyway. + return snapshot.selector.owner; + } } } diff --git a/packages/relay-runtime/store/__tests__/RelayModernEnvironment-WithOperationTracker-test.js b/packages/relay-runtime/store/__tests__/RelayModernEnvironment-WithOperationTracker-test.js index 4c20bc23d0e62..048d2ed5e2f3b 100644 --- a/packages/relay-runtime/store/__tests__/RelayModernEnvironment-WithOperationTracker-test.js +++ b/packages/relay-runtime/store/__tests__/RelayModernEnvironment-WithOperationTracker-test.js @@ -14,6 +14,7 @@ import type {NormalizationRootNode} from '../../util/NormalizationNode'; const {graphql} = require('../../query/GraphQLTag'); +const RelayFeatureFlags = require('../../util/RelayFeatureFlags'); const { createOperationDescriptor, } = require('../RelayModernOperationDescriptor'); @@ -28,412 +29,110 @@ const {disallowWarnings} = require('relay-test-utils-internal'); disallowWarnings(); -describe('RelayModernEnvironment with RelayOperationTracker', () => { - let tracker; - let environment; - let QueryOperation1; - let QueryOperation2; - let MutationOperation; - let operationLoader: { - get: (reference: mixed) => ?NormalizationRootNode, - load: JestMockFn<$ReadOnlyArray, Promise>, - }; - - beforeEach(() => { - const Query1 = graphql` - query RelayModernEnvironmentWithOperationTrackerTest1Query($id: ID) - @relay_test_operation { - node(id: $id) { - ... on Feedback { - id - body { - text - } - comments { - edges { - node { - id - message { - text +describe.each([true, false])( + 'RelayModernEnvironment with RelayOperationTracker with ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION=%p', + looseAttribution => { + let tracker; + let environment; + let QueryOperation1; + let QueryOperation2; + let MutationOperation; + let operationLoader: { + get: (reference: mixed) => ?NormalizationRootNode, + load: JestMockFn<$ReadOnlyArray, Promise>, + }; + + beforeEach(() => { + RelayFeatureFlags.ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION = + looseAttribution; + const Query1 = graphql` + query RelayModernEnvironmentWithOperationTrackerTest1Query($id: ID) + @relay_test_operation { + node(id: $id) { + ... on Feedback { + id + body { + text + } + comments { + edges { + node { + id + message { + text + } } } } } } } - } - `; + `; - const Query2 = graphql` - query RelayModernEnvironmentWithOperationTrackerTest2Query($id: ID) - @relay_test_operation { - node(id: $id) { - id - } - } - `; - - const Mutation1 = graphql` - mutation RelayModernEnvironmentWithOperationTrackerTest1Mutation( - $input: CommentCreateInput - ) @relay_test_operation { - commentCreate(input: $input) { - comment { - id - message { - text - } - } - feedback { - id - body { - text - } - } - } - } - `; - - QueryOperation1 = createOperationDescriptor(Query1, {id: '1'}); - QueryOperation2 = createOperationDescriptor(Query2, {id: '2'}); - MutationOperation = createOperationDescriptor(Mutation1, {id: '1'}); - operationLoader = { - load: jest.fn(), - get: jest.fn(), - }; - tracker = new RelayOperationTracker(); - environment = createMockEnvironment({ - operationTracker: tracker, - operationLoader, - }); - }); - - it('should return an instance of tracker', () => { - expect(environment.getOperationTracker()).toBe(tracker); - }); - - it('should have operation tracker and operations should not be affected', () => { - invariant(tracker != null, 'Tracker should be defined'); - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation2.request), - ).toBe(null); - }); - - it('sh©ould return a promise when there are pending operations that are affecting the owner', () => { - invariant(tracker != null, 'Tracker should be defined'); - environment - .execute({ - operation: QueryOperation1, - }) - .subscribe({}); - - // Note: Why do we need to `subscribe` here (and in other places)? - // We need to subscribe a fragment (with the owner) in order to simulate - // scenario when the fragment is updated - we need to update - // OperationTracker, and mark this owner as pending, or completed - if an - // operation that initiated the change is completed. - environment.subscribe( - environment.lookup(QueryOperation1.fragment), - jest.fn(), - ); - - const FEEDBACK_ID = 'my-feedback-id'; - - environment.mock.resolve( - QueryOperation1, - MockPayloadGenerator.generate(QueryOperation1, { - Feedback() { - return { - id: FEEDBACK_ID, - }; - }, - }), - ); - - environment - .executeMutation({ - operation: MutationOperation, - }) - .subscribe({}); - - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - - // This mutation is changing the same feedback object, so the owner (QueryOperation1) - // will be affected by this operation - environment.mock.nextValue( - MutationOperation, - MockPayloadGenerator.generate(MutationOperation, { - Feedback(context) { - return { - id: FEEDBACK_ID, - body: { - text: 'Updated text', - }, - }; - }, - }), - ); - - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request) - ?.promise, - ).toBeInstanceOf(Promise); - - // Complete the mutation - environment.mock.complete(MutationOperation.request.node); - - // There should be no pending operations affecting the owner, - // after the mutation is completed - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - }); - - it('should not have pending operation affecting the owner, if owner does not have subscriptions', () => { - invariant(tracker != null, 'Tracker should be defined'); - environment - .execute({ - operation: QueryOperation1, - }) - .subscribe({}); - - const FEEDBACK_ID = 'my-feedback-id'; - - environment.mock.resolve( - QueryOperation1, - MockPayloadGenerator.generate(QueryOperation1, { - Feedback() { - return { - id: FEEDBACK_ID, - }; - }, - }), - ); - - environment - .executeMutation({ - operation: MutationOperation, - }) - .subscribe({}); - - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - - // This mutation will update the same data as the QueryOperation1 - // but since there are not subscriptions that have owner QueryOperation1 - // it should not be affected - environment.mock.nextValue( - MutationOperation, - MockPayloadGenerator.generate(MutationOperation, { - Feedback(context) { - return { - id: FEEDBACK_ID, - body: { - text: 'Updated text', - }, - }; - }, - }), - ); - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - }); - - it('should return a promise for operation affecting owner that resolves when operation completes', () => { - invariant(tracker != null, 'Tracker should be defined'); - environment - .execute({ - operation: QueryOperation1, - }) - .subscribe({}); - - environment.subscribe( - environment.lookup(QueryOperation1.fragment), - jest.fn(), - ); - - const FEEDBACK_ID = 'my-feedback-id'; - - environment.mock.resolve( - QueryOperation1, - MockPayloadGenerator.generate(QueryOperation1, { - Feedback() { - return { - id: FEEDBACK_ID, - }; - }, - }), - ); - - // Let's start mutation - environment - .executeMutation({ - operation: MutationOperation, - }) - .subscribe({}); - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - environment.mock.nextValue( - MutationOperation, - MockPayloadGenerator.generate(MutationOperation, { - Feedback() { - return { - id: FEEDBACK_ID, - body: { - text: 'Updated text', - }, - }; - }, - }), - ); - const result = tracker.getPendingOperationsAffectingOwner( - QueryOperation1.request, - ); - - invariant(result != null, 'Expected to have promise for operation'); - const promiseCallback = jest.fn<[void], mixed>(); - // $FlowFixMe[unused-promise] - result.promise.then(promiseCallback); - expect(promiseCallback).not.toBeCalled(); - environment.mock.complete(MutationOperation.request.node); - jest.runAllTimers(); - expect(promiseCallback).toBeCalled(); - }); - - it('pending queries that did not change the data should not affect the owner', () => { - invariant(tracker != null, 'Tracker should be defined'); - // Send the first query - environment - .execute({ - operation: QueryOperation1, - }) - .subscribe({}); - - environment.subscribe( - environment.lookup(QueryOperation1.fragment), - jest.fn(), - ); - - // Send the second query - environment - .execute({ - operation: QueryOperation2, - }) - .subscribe({}); - - environment.subscribe( - environment.lookup(QueryOperation2.fragment), - jest.fn(), - ); - - environment.mock.resolve( - QueryOperation1, - MockPayloadGenerator.generate(QueryOperation1, { - Feedback() { - return { - id: 'feedback-id-1', - }; - }, - }), - ); - - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - - environment.mock.nextValue( - QueryOperation2, - MockPayloadGenerator.generate(QueryOperation2, { - Node() { - return { - __typename: 'Feedback', - id: 'feedback-id-2', - }; - }, - }), - ); - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); - }); - - describe('with @match', () => { - it('should return a promise for affecting operations', () => { - //const {Query, Mutation, FeedbackFragment} = - const Query = graphql` - query RelayModernEnvironmentWithOperationTrackerTestQuery($id: ID) + const Query2 = graphql` + query RelayModernEnvironmentWithOperationTrackerTest2Query($id: ID) @relay_test_operation { node(id: $id) { - ...RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment - } - } - `; - - graphql` - fragment RelayModernEnvironmentWithOperationTrackerTestPlainUserNameRenderer_name on PlainUserNameRenderer { - plaintext - data { - text - } - } - `; - graphql` - fragment RelayModernEnvironmentWithOperationTrackerTestMarkdownUserNameRenderer_name on MarkdownUserNameRenderer { - markdown - data { - markup - } - } - `; - - const FeedbackFragment = graphql` - fragment RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment on Feedback { - id - body { - text - } - author { - __typename - nameRenderer @match { - ...RelayModernEnvironmentWithOperationTrackerTestPlainUserNameRenderer_name - @module(name: "PlainUserNameRenderer.react") - ...RelayModernEnvironmentWithOperationTrackerTestMarkdownUserNameRenderer_name - @module(name: "MarkdownUserNameRenderer.react") - } - plainNameRenderer: nameRenderer - @match( - key: "RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment_plainNameRenderer" - ) { - ...RelayModernEnvironmentWithOperationTrackerTestPlainUserNameRenderer_name - @module(name: "PlainUserNameRenderer.react") - } + id } } `; - const Mutation = graphql` - mutation RelayModernEnvironmentWithOperationTrackerTestMutation( + const Mutation1 = graphql` + mutation RelayModernEnvironmentWithOperationTrackerTest1Mutation( $input: CommentCreateInput ) @relay_test_operation { commentCreate(input: $input) { + comment { + id + message { + text + } + } feedback { - ...RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment + id + lastName + body { + text + } } } } `; - QueryOperation1 = createOperationDescriptor(Query, {id: '1'}); - MutationOperation = createOperationDescriptor(Mutation, {id: '1'}); + QueryOperation1 = createOperationDescriptor(Query1, {id: '1'}); + QueryOperation2 = createOperationDescriptor(Query2, {id: '2'}); + MutationOperation = createOperationDescriptor(Mutation1, {id: '1'}); + operationLoader = { + load: jest.fn(), + get: jest.fn(), + }; + tracker = new RelayOperationTracker(); + environment = createMockEnvironment({ + operationTracker: tracker, + operationLoader, + }); + }); + afterEach(() => { + RelayFeatureFlags.ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION = false; + }); + + it('should return an instance of tracker', () => { + expect(environment.getOperationTracker()).toBe(tracker); + }); + + it('should have operation tracker and operations should not be affected', () => { + invariant(tracker != null, 'Tracker should be defined'); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation2.request), + ).toBe(null); + }); + + it('should return a promise when there are pending operations that are affecting the owner', () => { invariant(tracker != null, 'Tracker should be defined'); environment .execute({ @@ -441,84 +140,455 @@ describe('RelayModernEnvironment with RelayOperationTracker', () => { }) .subscribe({}); - const FEEDBACK_ID = 'my-feedback-id'; - + // Note: Why do we need to `subscribe` here (and in other places)? + // We need to subscribe a fragment (with the owner) in order to simulate + // scenario when the fragment is updated - we need to update + // OperationTracker, and mark this owner as pending, or completed - if an + // operation that initiated the change is completed. environment.subscribe( environment.lookup(QueryOperation1.fragment), jest.fn(), ); - environment.subscribe( - environment.lookup( - createReaderSelector( - FeedbackFragment, - FEEDBACK_ID, - QueryOperation1.request.variables, - QueryOperation1.request, - ), - ), - jest.fn(), + + const FEEDBACK_ID = 'my-feedback-id'; + + environment.mock.resolve( + QueryOperation1, + MockPayloadGenerator.generate(QueryOperation1, { + Feedback() { + return { + id: FEEDBACK_ID, + }; + }, + }), ); - operationLoader.load.mockImplementation(() => Promise.resolve()); - environment.mock.resolve(QueryOperation1.request.node, { - data: { - node: { - __typename: 'Feedback', - id: FEEDBACK_ID, - body: { - text: '', - }, - author: { - __typename: 'User', - nameRenderer: { - __typename: 'MarkdownUserNameRenderer', - markdown: 'mock value', - data: { - markup: 'mock value', - }, - __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: - '', - __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: - '', - }, - plainNameRenderer: { - __typename: 'PlainUserNameRenderer', - __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: - '', - __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: - '', + environment + .executeMutation({ + operation: MutationOperation, + }) + .subscribe({}); + + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + + // This mutation is changing the same feedback object, so the owner (QueryOperation1) + // will be affected by this operation + environment.mock.nextValue( + MutationOperation, + MockPayloadGenerator.generate(MutationOperation, { + Feedback(context) { + return { + id: FEEDBACK_ID, + body: { + text: 'Updated text', }, - id: '', - }, + }; }, - }, - }); - expect(operationLoader.load).toBeCalled(); - operationLoader.load.mockClear(); + }), + ); - // We still processing follow-up payloads for the initial query expect( tracker.getPendingOperationsAffectingOwner(QueryOperation1.request) ?.promise, ).toBeInstanceOf(Promise); - jest.runAllTimers(); - // All followup completed, operation tracker should be completed + // Complete the mutation + environment.mock.complete(MutationOperation.request.node); + + // There should be no pending operations affecting the owner, + // after the mutation is completed expect( tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), ).toBe(null); + }); + + it('should not have pending operation affecting the owner, if owner does not have subscriptions', () => { + invariant(tracker != null, 'Tracker should be defined'); + environment + .execute({ + operation: QueryOperation1, + }) + .subscribe({}); + + const FEEDBACK_ID = 'my-feedback-id'; + + environment.mock.resolve( + QueryOperation1, + MockPayloadGenerator.generate(QueryOperation1, { + Feedback() { + return { + id: FEEDBACK_ID, + }; + }, + }), + ); - // Send the mutation environment .executeMutation({ operation: MutationOperation, }) .subscribe({}); - environment.mock.nextValue(MutationOperation, { - data: { - commentCreate: { - feedback: { + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + + // This mutation will update the same data as the QueryOperation1 + // but since there are not subscriptions that have owner QueryOperation1 + // it should not be affected + environment.mock.nextValue( + MutationOperation, + MockPayloadGenerator.generate(MutationOperation, { + Feedback(context) { + return { + id: FEEDBACK_ID, + body: { + text: 'Updated text', + }, + }; + }, + }), + ); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + }); + + it('should return a promise for operation affecting owner that resolves when operation completes', () => { + invariant(tracker != null, 'Tracker should be defined'); + environment + .execute({ + operation: QueryOperation1, + }) + .subscribe({}); + + environment.subscribe( + environment.lookup(QueryOperation1.fragment), + jest.fn(), + ); + + const FEEDBACK_ID = 'my-feedback-id'; + + environment.mock.resolve( + QueryOperation1, + MockPayloadGenerator.generate(QueryOperation1, { + Feedback() { + return { + id: FEEDBACK_ID, + }; + }, + }), + ); + + // Let's start mutation + environment + .executeMutation({ + operation: MutationOperation, + }) + .subscribe({}); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + environment.mock.nextValue( + MutationOperation, + MockPayloadGenerator.generate(MutationOperation, { + Feedback() { + return { + id: FEEDBACK_ID, + body: { + text: 'Updated text', + }, + }; + }, + }), + ); + const result = tracker.getPendingOperationsAffectingOwner( + QueryOperation1.request, + ); + + invariant(result != null, 'Expected to have promise for operation'); + const promiseCallback = jest.fn<[void], mixed>(); + // $FlowFixMe[unused-promise] + result.promise.then(promiseCallback); + expect(promiseCallback).not.toBeCalled(); + environment.mock.complete(MutationOperation.request.node); + jest.runAllTimers(); + expect(promiseCallback).toBeCalled(); + }); + + it('pending queries that did not change the data should not affect the owner', () => { + invariant(tracker != null, 'Tracker should be defined'); + // Send the first query + environment.execute({operation: QueryOperation1}).subscribe({}); + + environment.subscribe( + environment.lookup(QueryOperation1.fragment), + jest.fn(), + ); + + // Send the second query + environment.execute({operation: QueryOperation2}).subscribe({}); + + environment.subscribe( + environment.lookup(QueryOperation2.fragment), + jest.fn(), + ); + + environment.mock.resolve( + QueryOperation1, + MockPayloadGenerator.generate(QueryOperation1, { + Feedback() { + return { + id: 'feedback-id-1', + }; + }, + }), + ); + + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + + environment.mock.nextValue( + QueryOperation2, + MockPayloadGenerator.generate(QueryOperation2, { + Node() { + return { + __typename: 'Feedback', + id: 'feedback-id-2', + }; + }, + }), + ); + + const operations = tracker.getPendingOperationsAffectingOwner( + QueryOperation1.request, + ); + + expect(operations).toBe(null); + }); + + // If a store update changes a record, that will force us to reread any fragment that + // read that ID. However, if that re-read results in identical data, we will not notify + // the subscribers. + // + // With ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION disabled (default) we also + // won't mark the store update as affecing the fragment. + // + // With ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION enabled we _will_ mark the + // store update as affecing the fragment. If this behavior is sufficient, it + // will allow us to support OperationTracker with lazy subscriptions that + // don't read eagerly. + it('pending queries that changed a record that was read, but not any fields', () => { + invariant(tracker != null, 'Tracker should be defined'); + environment.execute({operation: QueryOperation1}).subscribe({}); + + const query1Subscription = jest.fn(); + + environment.subscribe( + environment.lookup(QueryOperation1.fragment), + query1Subscription, + ); + + const FEEDBACK_ID = 'my-feedback-id'; + + environment.mock.resolve( + QueryOperation1, + MockPayloadGenerator.generate(QueryOperation1, { + Feedback() { + return {id: FEEDBACK_ID}; + }, + }), + ); + + expect(query1Subscription).toHaveBeenCalledTimes(1); + + // Let's start mutation + environment.executeMutation({operation: MutationOperation}).subscribe({}); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + environment.mock.nextValue( + MutationOperation, + MockPayloadGenerator.generate(MutationOperation, { + Feedback() { + return { + id: FEEDBACK_ID, + // This field changed on this record but, Query1 does not actually + // read it. This should mean that Query1 will get re-read, but + // should not actually trigger an update. + lastName: 'CHANGED', + }; + }, + }), + ); + + // Becuase `lastName` was not read by Query1, the subscription should not have notified. + expect(query1Subscription).toHaveBeenCalledTimes(1); + + const result = tracker.getPendingOperationsAffectingOwner( + QueryOperation1.request, + ); + + if (looseAttribution) { + invariant( + result != null, + `Expected to have promise for operation due to overlap on ${FEEDBACK_ID}.`, + ); + } else { + expect(result).toBe(null); + } + }); + + it('pending queries that changed ROOT_ID, but not other records read by the subscribed fragment', () => { + invariant(tracker != null, 'Tracker should be defined'); + environment.execute({operation: QueryOperation1}).subscribe({}); + + const query1Subscription = jest.fn(); + + environment.subscribe( + environment.lookup(QueryOperation1.fragment), + query1Subscription, + ); + + const FEEDBACK_ID = 'my-feedback-id'; + + environment.mock.resolve( + QueryOperation1, + MockPayloadGenerator.generate(QueryOperation1, { + Feedback() { + return {id: FEEDBACK_ID}; + }, + }), + ); + + expect(query1Subscription).toHaveBeenCalledTimes(1); + + // Let's start mutation + environment.executeMutation({operation: MutationOperation}).subscribe({}); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + + const payload = MockPayloadGenerator.generate(MutationOperation, { + Mutation() { + return {commentCreate: null}; + }, + }); + environment.mock.nextValue(MutationOperation, payload); + + expect(query1Subscription).toHaveBeenCalledTimes(1); + + const result = tracker.getPendingOperationsAffectingOwner( + QueryOperation1.request, + ); + + // Loose attribution should ignore changes to ROOT_ID, and without loose + // attribution there should be no changes to the read fragment data. + expect(result).toBe(null); + }); + + describe('with @match', () => { + it('should return a promise for affecting operations', () => { + //const {Query, Mutation, FeedbackFragment} = + const Query = graphql` + query RelayModernEnvironmentWithOperationTrackerTestQuery($id: ID) + @relay_test_operation { + node(id: $id) { + ...RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment + } + } + `; + + graphql` + fragment RelayModernEnvironmentWithOperationTrackerTestPlainUserNameRenderer_name on PlainUserNameRenderer { + plaintext + data { + text + } + } + `; + graphql` + fragment RelayModernEnvironmentWithOperationTrackerTestMarkdownUserNameRenderer_name on MarkdownUserNameRenderer { + markdown + data { + markup + } + } + `; + + const FeedbackFragment = graphql` + fragment RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment on Feedback { + id + body { + text + } + author { + __typename + nameRenderer @match { + ...RelayModernEnvironmentWithOperationTrackerTestPlainUserNameRenderer_name + @module(name: "PlainUserNameRenderer.react") + ...RelayModernEnvironmentWithOperationTrackerTestMarkdownUserNameRenderer_name + @module(name: "MarkdownUserNameRenderer.react") + } + plainNameRenderer: nameRenderer + @match( + key: "RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment_plainNameRenderer" + ) { + ...RelayModernEnvironmentWithOperationTrackerTestPlainUserNameRenderer_name + @module(name: "PlainUserNameRenderer.react") + } + } + } + `; + + const Mutation = graphql` + mutation RelayModernEnvironmentWithOperationTrackerTestMutation( + $input: CommentCreateInput + ) @relay_test_operation { + commentCreate(input: $input) { + feedback { + ...RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment + } + } + } + `; + + QueryOperation1 = createOperationDescriptor(Query, {id: '1'}); + MutationOperation = createOperationDescriptor(Mutation, {id: '1'}); + + invariant(tracker != null, 'Tracker should be defined'); + environment + .execute({ + operation: QueryOperation1, + }) + .subscribe({}); + + const FEEDBACK_ID = 'my-feedback-id'; + + environment.subscribe( + environment.lookup(QueryOperation1.fragment), + jest.fn(), + ); + environment.subscribe( + environment.lookup( + createReaderSelector( + FeedbackFragment, + FEEDBACK_ID, + QueryOperation1.request.variables, + QueryOperation1.request, + ), + ), + jest.fn(), + ); + + operationLoader.load.mockImplementation(() => Promise.resolve()); + environment.mock.resolve(QueryOperation1.request.node, { + data: { + node: { + __typename: 'Feedback', id: FEEDBACK_ID, body: { text: '', @@ -526,7 +596,11 @@ describe('RelayModernEnvironment with RelayOperationTracker', () => { author: { __typename: 'User', nameRenderer: { - __typename: 'PlainUserNameRenderer', + __typename: 'MarkdownUserNameRenderer', + markdown: 'mock value', + data: { + markup: 'mock value', + }, __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: '', __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: @@ -534,29 +608,81 @@ describe('RelayModernEnvironment with RelayOperationTracker', () => { }, plainNameRenderer: { __typename: 'PlainUserNameRenderer', - __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment_plainNameRenderer: + __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: '', - __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment_plainNameRenderer: + __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: '', }, id: '', }, }, }, - }, + }); + expect(operationLoader.load).toBeCalled(); + operationLoader.load.mockClear(); + + // We still processing follow-up payloads for the initial query + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request) + ?.promise, + ).toBeInstanceOf(Promise); + jest.runAllTimers(); + + // All followup completed, operation tracker should be completed + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); + + // Send the mutation + environment + .executeMutation({ + operation: MutationOperation, + }) + .subscribe({}); + + environment.mock.nextValue(MutationOperation, { + data: { + commentCreate: { + feedback: { + id: FEEDBACK_ID, + body: { + text: '', + }, + author: { + __typename: 'User', + nameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: + '', + __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment: + '', + }, + plainNameRenderer: { + __typename: 'PlainUserNameRenderer', + __module_component_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment_plainNameRenderer: + '', + __module_operation_RelayModernEnvironmentWithOperationTrackerTestFeedbackFragment_plainNameRenderer: + '', + }, + id: '', + }, + }, + }, + }, + }); + + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request) + ?.promise, + ).toBeInstanceOf(Promise); + + environment.mock.complete(MutationOperation); + expect(operationLoader.load).toBeCalled(); + jest.runAllTimers(); + expect( + tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), + ).toBe(null); }); - - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request) - ?.promise, - ).toBeInstanceOf(Promise); - - environment.mock.complete(MutationOperation); - expect(operationLoader.load).toBeCalled(); - jest.runAllTimers(); - expect( - tracker.getPendingOperationsAffectingOwner(QueryOperation1.request), - ).toBe(null); }); - }); -}); + }, +); diff --git a/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentWithOperationTrackerTest1Mutation.graphql.js b/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentWithOperationTrackerTest1Mutation.graphql.js index 30008cbf3f67a..7898187b4fd28 100644 --- a/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentWithOperationTrackerTest1Mutation.graphql.js +++ b/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentWithOperationTrackerTest1Mutation.graphql.js @@ -6,7 +6,7 @@ * * @oncall relay * - * @generated SignedSource<<0d585e4fcf0408c51ad2551e13a03a93>> + * @generated SignedSource<<4ed5f99cf70e4f1f44b9cc579229be03>> * @flow * @lightSyntaxTransform * @nogrep @@ -44,6 +44,7 @@ export type RelayModernEnvironmentWithOperationTrackerTest1Mutation$data = {| +text: ?string, |}, +id: string, + +lastName: ?string, |}, |}, |}; @@ -123,6 +124,13 @@ v3 = [ "plural": false, "selections": [ (v1/*: any*/), + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "lastName", + "storageKey": null + }, { "alias": null, "args": null, @@ -176,7 +184,7 @@ return { "selections": (v3/*: any*/) }, "params": { - "cacheID": "33e7acb2141a8b4cc30d8eb08feb28b2", + "cacheID": "bc860715cdbb5c16bfec84cccf10eb67", "id": null, "metadata": { "relayTestingSelectionTypeInfo": { @@ -203,18 +211,19 @@ return { }, "commentCreate.feedback.body": (v5/*: any*/), "commentCreate.feedback.body.text": (v6/*: any*/), - "commentCreate.feedback.id": (v4/*: any*/) + "commentCreate.feedback.id": (v4/*: any*/), + "commentCreate.feedback.lastName": (v6/*: any*/) } }, "name": "RelayModernEnvironmentWithOperationTrackerTest1Mutation", "operationKind": "mutation", - "text": "mutation RelayModernEnvironmentWithOperationTrackerTest1Mutation(\n $input: CommentCreateInput\n) {\n commentCreate(input: $input) {\n comment {\n id\n message {\n text\n }\n }\n feedback {\n id\n body {\n text\n }\n }\n }\n}\n" + "text": "mutation RelayModernEnvironmentWithOperationTrackerTest1Mutation(\n $input: CommentCreateInput\n) {\n commentCreate(input: $input) {\n comment {\n id\n message {\n text\n }\n }\n feedback {\n id\n lastName\n body {\n text\n }\n }\n }\n}\n" } }; })(); if (__DEV__) { - (node/*: any*/).hash = "191ed594a345f64de3ccd4b8bc51e924"; + (node/*: any*/).hash = "6a10ff9c1fc045181ae2f8edcaf0e88a"; } module.exports = ((node/*: any*/)/*: Mutation< diff --git a/packages/relay-runtime/store/hasSignificantOverlappingIDs.js b/packages/relay-runtime/store/hasSignificantOverlappingIDs.js new file mode 100644 index 0000000000000..e8285204c5d84 --- /dev/null +++ b/packages/relay-runtime/store/hasSignificantOverlappingIDs.js @@ -0,0 +1,38 @@ +/** + * 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 strict-local + * @format + * @oncall relay + */ + +'use strict'; + +import type {DataIDSet} from './RelayStoreTypes'; + +const {ROOT_ID} = require('./RelayStoreUtils'); +const {VIEWER_ID} = require('./ViewerPattern'); + +const ITERATOR_KEY = Symbol.iterator; + +function hasSignificantOverlappingIDs( + seenRecords: DataIDSet, + updatedRecordIDs: DataIDSet, +): boolean { + // $FlowFixMe[incompatible-use]: Set is an iterable type, accessing its iterator is allowed. + const iterator = seenRecords[ITERATOR_KEY](); + let next = iterator.next(); + while (!next.done) { + const key = next.value; + if (updatedRecordIDs.has(key) && key !== ROOT_ID && key !== VIEWER_ID) { + return true; + } + next = iterator.next(); + } + return false; +} + +module.exports = hasSignificantOverlappingIDs; diff --git a/packages/relay-runtime/util/RelayFeatureFlags.js b/packages/relay-runtime/util/RelayFeatureFlags.js index a6c401c95b9fb..c7157e3c8b727 100644 --- a/packages/relay-runtime/util/RelayFeatureFlags.js +++ b/packages/relay-runtime/util/RelayFeatureFlags.js @@ -32,6 +32,15 @@ export type FeatureFlags = { USE_REACT_CACHE_LEGACY_TIMEOUTS: boolean, ENABLE_QUERY_RENDERER_SET_STATE_PREVENTION: boolean, LOG_MISSING_RECORDS_IN_PROD: boolean, + + // Configure RelayStoreSubscriptions to mark a subscription as affected by an + // update if there are any overlapping IDs other than ROOT_ID or VIWER_ID, + // even if none of the read fields were affected. The strict behavior (current + // default) requires eagerly reading fragments as they change which is + // incompatible with lazily notifying React of updats using `setState(() => + // read())`, so we are experimenting with this loose behavior which should be + // more compatible. + ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION: boolean, }; const RelayFeatureFlags: FeatureFlags = { @@ -53,6 +62,7 @@ const RelayFeatureFlags: FeatureFlags = { USE_REACT_CACHE_LEGACY_TIMEOUTS: true, ENABLE_QUERY_RENDERER_SET_STATE_PREVENTION: false, LOG_MISSING_RECORDS_IN_PROD: false, + ENABLE_LOOSE_SUBSCRIPTION_ATTRIBUTION: false, }; module.exports = RelayFeatureFlags;