From cd1e9ae06d114c3e353d384ccbe956e6800a8679 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Mon, 11 Jul 2022 12:43:24 -0700 Subject: [PATCH] Fix MutationHandlers on field with args Summary: This should fix https://github.com/facebook/relay/issues/3457 The bug is in calling `record.getLinkedRecord(payload.fieldKey, payload.args)`. `getLinkedRecord` would append the serialized `args` to `fieldKey` as the final field name. However, `payload.fieldKey` already contains serialized arguments when it's populated using `getStorageKey` in `RelayRepsonseNormalizer`. We were duplicating the arguments in the field name. Removing the `payload.args` to `record.getLinkedRecord` fixes the issue. Reviewed By: voideanvalue Differential Revision: D37732466 fbshipit-source-id: 9fc5a46643e463f265633b8390998c01e2f70b36 --- .../handlers/connection/MutationHandlers.js | 8 +- ...uteMutationWithDeclarativeMutation-test.js | 198 ++++++++++++++++ ...tAppendCommentsWithArgsMutation.graphql.js | 221 ++++++++++++++++++ .../testschema.graphql | 1 + 4 files changed, 424 insertions(+), 4 deletions(-) create mode 100644 packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation.graphql.js diff --git a/packages/relay-runtime/handlers/connection/MutationHandlers.js b/packages/relay-runtime/handlers/connection/MutationHandlers.js index 1ab3050826072..8ab580112bd41 100644 --- a/packages/relay-runtime/handlers/connection/MutationHandlers.js +++ b/packages/relay-runtime/handlers/connection/MutationHandlers.js @@ -104,11 +104,11 @@ function edgeUpdater( ); let singleServerEdge, serverEdges; try { - singleServerEdge = record.getLinkedRecord(payload.fieldKey, payload.args); + singleServerEdge = record.getLinkedRecord(payload.fieldKey); } catch {} if (!singleServerEdge) { try { - serverEdges = record.getLinkedRecords(payload.fieldKey, payload.args); + serverEdges = record.getLinkedRecords(payload.fieldKey); } catch {} } if (singleServerEdge == null && serverEdges == null) { @@ -181,11 +181,11 @@ function nodeUpdater( let singleServerNode; let serverNodes; try { - singleServerNode = record.getLinkedRecord(payload.fieldKey, payload.args); + singleServerNode = record.getLinkedRecord(payload.fieldKey); } catch {} if (!singleServerNode) { try { - serverNodes = record.getLinkedRecords(payload.fieldKey, payload.args); + serverNodes = record.getLinkedRecords(payload.fieldKey); } catch {} } if (singleServerNode == null && serverNodes == null) { diff --git a/packages/relay-runtime/store/__tests__/RelayModernEnvironment-ExecuteMutationWithDeclarativeMutation-test.js b/packages/relay-runtime/store/__tests__/RelayModernEnvironment-ExecuteMutationWithDeclarativeMutation-test.js index 3fc89992dcbaa..74313ef1da7ec 100644 --- a/packages/relay-runtime/store/__tests__/RelayModernEnvironment-ExecuteMutationWithDeclarativeMutation-test.js +++ b/packages/relay-runtime/store/__tests__/RelayModernEnvironment-ExecuteMutationWithDeclarativeMutation-test.js @@ -25,11 +25,13 @@ const { const RelayModernStore = require('../RelayModernStore'); const RelayRecordSource = require('../RelayRecordSource'); const { + disallowConsoleErrors, disallowWarnings, expectWarningWillFire, } = require('relay-test-utils-internal'); disallowWarnings(); +disallowConsoleErrors(); describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])( 'deleteFromStore', @@ -633,12 +635,14 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])( let store; let AppendCommentMutation; let AppendCommentsMutation; + let AppendCommentsWithArgsMutation; let PrependCommentMutation; let PrependCommentsMutation; let DeleteCommentMutation; let DeleteCommentsMutation; let appendOperation; let appendMultipleOperation; + let appendMultipleWithArgsOperation; let prependOperation; let prependMultipleOperation; let deleteOperation; @@ -702,6 +706,26 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])( } `; + AppendCommentsWithArgsMutation = graphql` + mutation RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation( + $connections: [ID!]! + $input: CommentCreateInput + $name: String! + ) { + commentCreate(input: $input) { + comment { + commentsFrom(name: $name) + @appendEdge(connections: $connections) { + cursor + node { + id + } + } + } + } + } + `; + PrependCommentMutation = graphql` mutation RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestPrependCommentMutation( $connections: [ID!]! @@ -770,6 +794,14 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])( input: {}, }, ); + appendMultipleWithArgsOperation = createOperationDescriptor( + AppendCommentsWithArgsMutation, + { + connections: [clientID], + name: 'Zuck', + input: {}, + }, + ); prependOperation = createOperationDescriptor(PrependCommentMutation, { connections: [clientID], input: {}, @@ -1218,6 +1250,172 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])( ]); }); + it('commits the mutation and inserts multiple comment edges on a field with args into the connection', () => { + const snapshot = environment.lookup(operation.fragment); + const callback = jest.fn(); + environment.subscribe(snapshot, callback); + + environment + .executeMutation({ + operation: appendMultipleWithArgsOperation, + }) + .subscribe(callbacks); + + callback.mockClear(); + subject.next({ + data: { + commentCreate: { + comment: { + id: 'unused-comment', + commentsFrom: [ + { + __typename: 'CommentsEdge', + cursor: 'node-append-1', + node: { + __typename: 'Comment', + id: 'node-append-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-append-2', + node: { + __typename: 'Comment', + id: 'node-append-2', + }, + }, + ], + }, + }, + }, + }); + subject.complete(); + + expect(error).not.toBeCalled(); + expect(complete).toBeCalled(); + expect(callback.mock.calls.length).toBe(1); + // $FlowExpectedError[incompatible-use] + expect(callback.mock.calls[0][0].data.node.comments.edges).toEqual([ + { + __typename: 'CommentsEdge', + cursor: 'cursor-1', + node: { + __typename: 'Comment', + id: 'node-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'cursor-2', + node: { + __typename: 'Comment', + id: 'node-2', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-append-1', + node: { + __typename: 'Comment', + id: 'node-append-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-append-2', + node: { + __typename: 'Comment', + id: 'node-append-2', + }, + }, + ]); + + environment + .executeMutation({ + operation: prependMultipleOperation, + }) + .subscribe(callbacks); + + callback.mockClear(); + subject.next({ + data: { + commentsCreate: { + feedbackCommentEdges: [ + { + __typename: 'CommentsEdge', + cursor: 'node-prepend-1', + node: { + __typename: 'Comment', + id: 'node-prepend-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-prepend-2', + node: { + __typename: 'Comment', + id: 'node-prepend-2', + }, + }, + ], + }, + }, + }); + subject.complete(); + expect(callback.mock.calls.length).toBe(1); + // $FlowExpectedError[incompatible-use] + expect(callback.mock.calls[0][0].data.node.comments.edges).toEqual([ + { + __typename: 'CommentsEdge', + cursor: 'node-prepend-2', + node: { + __typename: 'Comment', + id: 'node-prepend-2', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-prepend-1', + node: { + __typename: 'Comment', + id: 'node-prepend-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'cursor-1', + node: { + __typename: 'Comment', + id: 'node-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'cursor-2', + node: { + __typename: 'Comment', + id: 'node-2', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-append-1', + node: { + __typename: 'Comment', + id: 'node-append-1', + }, + }, + { + __typename: 'CommentsEdge', + cursor: 'node-append-2', + node: { + __typename: 'Comment', + id: 'node-append-2', + }, + }, + ]); + }); + it('inserts an comment edge during optmistic update, and reverts and inserts new edge when server payload resolves', () => { const snapshot = environment.lookup(operation.fragment); const callback = jest.fn(); diff --git a/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation.graphql.js b/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation.graphql.js new file mode 100644 index 0000000000000..9d872dc232dba --- /dev/null +++ b/packages/relay-runtime/store/__tests__/__generated__/RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation.graphql.js @@ -0,0 +1,221 @@ +/** + * 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. + * + * @generated SignedSource<> + * @flow + * @lightSyntaxTransform + * @nogrep + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ConcreteRequest, Mutation } from 'relay-runtime'; +export type CommentCreateInput = {| + feedback?: ?CommentfeedbackFeedback, + feedbackId?: ?string, +|}; +export type CommentfeedbackFeedback = {| + comment?: ?FeedbackcommentComment, +|}; +export type FeedbackcommentComment = {| + feedback?: ?CommentfeedbackFeedback, +|}; +export type RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation$variables = {| + connections: $ReadOnlyArray, + input?: ?CommentCreateInput, + name: string, +|}; +export type RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation$data = {| + +commentCreate: ?{| + +comment: ?{| + +commentsFrom: ?$ReadOnlyArray, + |}, + |}, +|}; +export type RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation = {| + response: RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation$data, + variables: RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation$variables, +|}; +*/ + +var node/*: ConcreteRequest*/ = (function(){ +var v0 = [ + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "connections" + }, + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "input" + }, + { + "defaultValue": null, + "kind": "LocalArgument", + "name": "name" + } +], +v1 = [ + { + "kind": "Variable", + "name": "input", + "variableName": "input" + } +], +v2 = [ + { + "kind": "Variable", + "name": "name", + "variableName": "name" + } +], +v3 = { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "id", + "storageKey": null +}, +v4 = { + "alias": null, + "args": (v2/*: any*/), + "concreteType": "CommentsEdge", + "kind": "LinkedField", + "name": "commentsFrom", + "plural": true, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "cursor", + "storageKey": null + }, + { + "alias": null, + "args": null, + "concreteType": "Comment", + "kind": "LinkedField", + "name": "node", + "plural": false, + "selections": [ + (v3/*: any*/) + ], + "storageKey": null + } + ], + "storageKey": null +}; +return { + "fragment": { + "argumentDefinitions": (v0/*: any*/), + "kind": "Fragment", + "metadata": null, + "name": "RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation", + "selections": [ + { + "alias": null, + "args": (v1/*: any*/), + "concreteType": "CommentCreateResponsePayload", + "kind": "LinkedField", + "name": "commentCreate", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "concreteType": "Comment", + "kind": "LinkedField", + "name": "comment", + "plural": false, + "selections": [ + (v4/*: any*/) + ], + "storageKey": null + } + ], + "storageKey": null + } + ], + "type": "Mutation", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": (v0/*: any*/), + "kind": "Operation", + "name": "RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation", + "selections": [ + { + "alias": null, + "args": (v1/*: any*/), + "concreteType": "CommentCreateResponsePayload", + "kind": "LinkedField", + "name": "commentCreate", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "concreteType": "Comment", + "kind": "LinkedField", + "name": "comment", + "plural": false, + "selections": [ + (v4/*: any*/), + { + "alias": null, + "args": (v2/*: any*/), + "filters": null, + "handle": "appendEdge", + "key": "", + "kind": "LinkedHandle", + "name": "commentsFrom", + "handleArgs": [ + { + "kind": "Variable", + "name": "connections", + "variableName": "connections" + } + ] + }, + (v3/*: any*/) + ], + "storageKey": null + } + ], + "storageKey": null + } + ] + }, + "params": { + "cacheID": "2b1c7fcb928d547f115912e724b19688", + "id": null, + "metadata": {}, + "name": "RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation", + "operationKind": "mutation", + "text": "mutation RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation(\n $input: CommentCreateInput\n $name: String!\n) {\n commentCreate(input: $input) {\n comment {\n commentsFrom(name: $name) {\n cursor\n node {\n id\n }\n }\n id\n }\n }\n}\n" + } +}; +})(); + +if (__DEV__) { + (node/*: any*/).hash = "7f8a8b0fb04d9fcfc4c3167b55ca7da7"; +} + +module.exports = ((node/*: any*/)/*: Mutation< + RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation$variables, + RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation$data, +>*/); diff --git a/packages/relay-test-utils-internal/testschema.graphql b/packages/relay-test-utils-internal/testschema.graphql index f4125574aa8fa..97919aebf3249 100644 --- a/packages/relay-test-utils-internal/testschema.graphql +++ b/packages/relay-test-utils-internal/testschema.graphql @@ -305,6 +305,7 @@ type Comment implements Node { subscribeStatus: String subscribers(first: Int): SubscribersConnection topLevelComments(first: Int): TopLevelCommentsConnection + commentsFrom(name: String!): [CommentsEdge] tracking: String url(relative: Boolean, site: String): String websites: [String]