Don't flatten fragment spreads with differing directives

Reviewed By: tyao1 Differential Revision: D35797243 fbshipit-source-id: 0c7e19e6d543b18556263bff8898dc583e62ee10
facebook · Apr 25, 2022 · 52af6a6 · 52af6a6
1 parent 0595e74
commit 52af6a6
Show file tree

Hide file tree

Showing 5 changed files with 221 additions and 3 deletions.
diff --git a/compiler/crates/graphql-ir/src/node_identifier.rs b/compiler/crates/graphql-ir/src/node_identifier.rs
@@ -71,6 +71,8 @@ impl<TBehavior: LocationAgnosticBehavior> NodeIdentifier<TBehavior> {
             (Selection::FragmentSpread(a), Selection::FragmentSpread(b)) => {
                 a.fragment.item == b.fragment.item
                     && a.arguments.location_agnostic_eq::<TBehavior>(&b.arguments)
+                    && a.directives
+                        .location_agnostic_eq::<TBehavior>(&b.directives)
             }
             (Selection::InlineFragment(a), Selection::InlineFragment(b)) => {
                 a.type_condition == b.type_condition
@@ -109,6 +111,8 @@ impl<TBehavior: LocationAgnosticBehavior> PartialEq for NodeIdentifier<TBehavior
             (NodeIdentifierInner::FragmentSpread(l), NodeIdentifierInner::FragmentSpread(r)) => {
                 l.fragment.item == r.fragment.item
                     && l.arguments.location_agnostic_eq::<TBehavior>(&r.arguments)
+                    && l.directives
+                        .location_agnostic_eq::<TBehavior>(&r.directives)
             }
             (NodeIdentifierInner::InlineFragment(l), NodeIdentifierInner::InlineFragment(r)) => {
                 l.type_condition == r.type_condition
@@ -138,6 +142,7 @@ impl<TBehavior: LocationAgnosticBehavior> Hash for NodeIdentifier<TBehavior> {
             NodeIdentifierInner::FragmentSpread(v) => {
                 v.fragment.item.hash(state);
                 v.arguments.location_agnostic_hash::<_, TBehavior>(state);
+                v.directives.location_agnostic_hash::<_, TBehavior>(state);
             }
             NodeIdentifierInner::InlineFragment(v) => {
                 v.type_condition.hash(state);

diff --git a/...elay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected b/...elay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected
@@ -0,0 +1,189 @@
+==================================== INPUT ====================================
+fragment relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name on User {
+  name
+}
+
+query relayResolversWithDifferentFieldArgsAreNotMerged_Query {
+  node(id: "SOME_ID") {
+    ... on User {
+      ten: pop_star_name(some_arg: 10)
+      twenty: pop_star_name(some_arg: 20)
+    }
+  }
+}
+
+# %extensions%
+
+extend type User {
+  pop_star_name(some_arg: Int!): String @relay_resolver(fragment_name: "relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name", import_path: "./path/to/PopStarNameResolver.js")
+}
+==================================== OUTPUT ===================================
+{
+  "fragment": {
+    "argumentDefinitions": [],
+    "kind": "Fragment",
+    "metadata": null,
+    "name": "relayResolversWithDifferentFieldArgsAreNotMerged_Query",
+    "selections": [
+      {
+        "alias": null,
+        "args": [
+          {
+            "kind": "Literal",
+            "name": "id",
+            "value": "SOME_ID"
+          }
+        ],
+        "concreteType": null,
+        "kind": "LinkedField",
+        "name": "node",
+        "plural": false,
+        "selections": [
+          {
+            "kind": "InlineFragment",
+            "selections": [
+              {
+                "alias": "ten",
+                "args": [
+                  {
+                    "kind": "Literal",
+                    "name": "some_arg",
+                    "value": 10
+                  }
+                ],
+                "fragment": {
+                  "args": null,
+                  "kind": "FragmentSpread",
+                  "name": "relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name"
+                },
+                "kind": "RelayResolver",
+                "name": "pop_star_name",
+                "resolverModule": require('PopStarNameResolver'),
+                "path": "node.ten"
+              },
+              {
+                "alias": "twenty",
+                "args": [
+                  {
+                    "kind": "Literal",
+                    "name": "some_arg",
+                    "value": 20
+                  }
+                ],
+                "fragment": {
+                  "args": null,
+                  "kind": "FragmentSpread",
+                  "name": "relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name"
+                },
+                "kind": "RelayResolver",
+                "name": "pop_star_name",
+                "resolverModule": require('PopStarNameResolver'),
+                "path": "node.twenty"
+              }
+            ],
+            "type": "User",
+            "abstractKey": null
+          }
+        ],
+        "storageKey": "node(id:\"SOME_ID\")"
+      }
+    ],
+    "type": "Query",
+    "abstractKey": null
+  },
+  "kind": "Request",
+  "operation": {
+    "argumentDefinitions": [],
+    "kind": "Operation",
+    "name": "relayResolversWithDifferentFieldArgsAreNotMerged_Query",
+    "selections": [
+      {
+        "alias": null,
+        "args": [
+          {
+            "kind": "Literal",
+            "name": "id",
+            "value": "SOME_ID"
+          }
+        ],
+        "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,
+                "kind": "ScalarField",
+                "name": "name",
+                "storageKey": null
+              }
+            ],
+            "type": "User",
+            "abstractKey": null
+          },
+          {
+            "alias": null,
+            "args": null,
+            "kind": "ScalarField",
+            "name": "id",
+            "storageKey": null
+          }
+        ],
+        "storageKey": "node(id:\"SOME_ID\")"
+      }
+    ]
+  },
+  "params": {
+    "id": null,
+    "metadata": {},
+    "name": "relayResolversWithDifferentFieldArgsAreNotMerged_Query",
+    "operationKind": "query",
+    "text": null
+  }
+}
+
+QUERY:
+
+query relayResolversWithDifferentFieldArgsAreNotMerged_Query {
+  node(id: "SOME_ID") {
+    __typename
+    ... on User {
+      ...relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name
+    }
+    id
+  }
+}
+
+fragment relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name on User {
+  name
+}
+
+
+{
+  "argumentDefinitions": [],
+  "kind": "Fragment",
+  "metadata": null,
+  "name": "relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name",
+  "selections": [
+    {
+      "alias": null,
+      "args": null,
+      "kind": "ScalarField",
+      "name": "name",
+      "storageKey": null
+    }
+  ],
+  "type": "User",
+  "abstractKey": null
+}
diff --git a/...relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.graphql b/...relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.graphql
@@ -0,0 +1,18 @@
+fragment relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name on User {
+  name
+}
+
+query relayResolversWithDifferentFieldArgsAreNotMerged_Query {
+  node(id: "SOME_ID") {
+    ... on User {
+      ten: pop_star_name(some_arg: 10)
+      twenty: pop_star_name(some_arg: 20)
+    }
+  }
+}
+
+# %extensions%
+
+extend type User {
+  pop_star_name(some_arg: Int!): String @relay_resolver(fragment_name: "relayResolversWithDifferentFieldArgsAreNotMerged_PopStarNameResolverFragment_name", import_path: "./path/to/PopStarNameResolver.js")
+}
diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts_test.rs b/compiler/crates/relay-compiler/tests/compile_relay_artifacts_test.rs
@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<dbdab2b591c1ebbc2ab40a34ecb37262>>
+ * @generated SignedSource<<261efcc28b37d85d772bfcf80464cfaf>>
  */
 
 mod compile_relay_artifacts;
@@ -1020,6 +1020,13 @@ fn relay_resolver_with_args() {
     test_fixture(transform_fixture, "relay-resolver-with-args.graphql", "compile_relay_artifacts/fixtures/relay-resolver-with-args.expected", input, expected);
 }
 
+#[test]
+fn relay_resolvers_with_different_field_args_are_not_merged() {
+    let input = include_str!("compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.graphql");
+    let expected = include_str!("compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected");
+    test_fixture(transform_fixture, "relay-resolvers-with-different-field-args-are-not-merged.graphql", "compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected", input, expected);
+}
+
 #[test]
 fn required_argument_not_passed_default_value() {
     let input = include_str!("compile_relay_artifacts/fixtures/required_argument_not_passed_default_value.graphql");

diff --git a/compiler/crates/relay-transforms/src/util.rs b/compiler/crates/relay-transforms/src/util.rs
@@ -98,7 +98,7 @@ lazy_static! {
         ProvidedVariableMetadata::directive_name(),
         FragmentAliasMetadata::directive_name(),
     ];
-    static ref DIRECTIVES_SKIPPED_IN_NODE_IDENTIFIER: [StringKey; 12] = [
+    static ref DIRECTIVES_SKIPPED_IN_NODE_IDENTIFIER: [StringKey; 11] = [
         *CLIENT_EXTENSION_DIRECTIVE_NAME,
         ConnectionMetadataDirective::directive_name(),
         *HANDLE_FIELD_DIRECTIVE_NAME,
@@ -109,7 +109,6 @@ lazy_static! {
         *REACT_FLIGHT_SCALAR_FLIGHT_FIELD_METADATA_KEY,
         ReactFlightLocalComponentsMetadata::directive_name(),
         *REQUIRED_DIRECTIVE_NAME,
-        RelayResolverSpreadMetadata::directive_name(),
         RelayClientComponentMetadata::directive_name(),
     ];
     static ref RELAY_CUSTOM_INLINE_FRAGMENT_DIRECTIVES: [StringKey; 7] = [