Validate conflicting selections in client fields

Summary: Moving the `validate_selection_conflict` before we are skipping the client extensions correctly validates conflicts in the client-only fields. Reviewed By: tyao1 Differential Revision: D37069206 fbshipit-source-id: e56349fa2a39047b38031722799d8522262d4f7b
facebook · Jun 13, 2022 · a86323f · a86323f
1 parent 1450153
commit a86323f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 72 deletions.
diff --git a/...r/tests/compile_relay_artifacts/fixtures/client-fields-overlapping-error.invalid.expected b/...r/tests/compile_relay_artifacts/fixtures/client-fields-overlapping-error.invalid.expected
@@ -1,4 +1,6 @@
 ==================================== INPUT ====================================
+# expected-to-throw
+
 query clientFieldsOverlappingErrorQuery {
   client_field(arg: "Value1")
   client_field(arg: "Value2")
@@ -9,72 +11,19 @@ query clientFieldsOverlappingErrorQuery {
 extend type Query {
   client_field(arg: String): String
 }
-==================================== OUTPUT ===================================
-{
-  "fragment": {
-    "argumentDefinitions": [],
-    "kind": "Fragment",
-    "metadata": null,
-    "name": "clientFieldsOverlappingErrorQuery",
-    "selections": [
-      {
-        "kind": "ClientExtension",
-        "selections": [
-          {
-            "alias": null,
-            "args": [
-              {
-                "kind": "Literal",
-                "name": "arg",
-                "value": "Value1"
-              }
-            ],
-            "kind": "ScalarField",
-            "name": "client_field",
-            "storageKey": "client_field(arg:\"Value1\")"
-          }
-        ]
-      }
-    ],
-    "type": "Query",
-    "abstractKey": null
-  },
-  "kind": "Request",
-  "operation": {
-    "argumentDefinitions": [],
-    "kind": "Operation",
-    "name": "clientFieldsOverlappingErrorQuery",
-    "selections": [
-      {
-        "kind": "ClientExtension",
-        "selections": [
-          {
-            "alias": null,
-            "args": [
-              {
-                "kind": "Literal",
-                "name": "arg",
-                "value": "Value1"
-              }
-            ],
-            "kind": "ScalarField",
-            "name": "client_field",
-            "storageKey": "client_field(arg:\"Value1\")"
-          }
-        ]
-      }
-    ]
-  },
-  "params": {
-    "cacheID": "94883f2a9cbd52c2f01b84f026506e89",
-    "id": null,
-    "metadata": {},
-    "name": "clientFieldsOverlappingErrorQuery",
-    "operationKind": "query",
-    "text": null
-  }
-}
+==================================== ERROR ====================================
+✖︎ Expected all fields on the same parent with the name or alias `client_field` to have the same argument values after applying fragment arguments. This field has the applied argument values: (arg: "Value1")
+
+  client-fields-overlapping-error.invalid.graphql:4:3
+    3 │ query clientFieldsOverlappingErrorQuery {
+    4 │   client_field(arg: "Value1")
+      │   ^^^^^^^^^^^^
+    5 │   client_field(arg: "Value2")
 
-QUERY:
+  ℹ︎ which conflicts with this field with applied argument values (arg: "Value2")
 
-Query Text is Empty.
+  client-fields-overlapping-error.invalid.graphql:5:3
+    4 │   client_field(arg: "Value1")
+    5 │   client_field(arg: "Value2")
+      │   ^^^^^^^^^^^^
+    6 │ }
diff --git a/...er/tests/compile_relay_artifacts/fixtures/client-fields-overlapping-error.invalid.graphql b/...er/tests/compile_relay_artifacts/fixtures/client-fields-overlapping-error.invalid.graphql
@@ -1,3 +1,5 @@
+# expected-to-throw
+
 query clientFieldsOverlappingErrorQuery {
   client_field(arg: "Value1")
   client_field(arg: "Value2")

diff --git a/compiler/crates/relay-transforms/src/apply_transforms.rs b/compiler/crates/relay-transforms/src/apply_transforms.rs
@@ -495,6 +495,11 @@ fn apply_operation_text_transforms(
     program = log_event.time("skip_unreachable_node_strict", || {
         skip_unreachable_node_strict(&program)
     })?;
+    log_event.time("validate_selection_conflict", || {
+        graphql_ir_validations::validate_selection_conflict::<RelayLocationAgnosticBehavior>(
+            &program, false,
+        )
+    })?;
     program = log_event.time("skip_client_extensions", || {
         skip_client_extensions(&program)
     });
@@ -506,11 +511,6 @@ fn apply_operation_text_transforms(
     program = log_event.time("skip_null_arguments_transform", || {
         skip_null_arguments_transform(&program)
     });
-    log_event.time("validate_selection_conflict", || {
-        graphql_ir_validations::validate_selection_conflict::<RelayLocationAgnosticBehavior>(
-            &program, false,
-        )
-    })?;
     log_event.time("flatten", || flatten(&mut program, false, true))?;
     program = log_event.time("validate_operation_variables", || {
         validate_operation_variables(&program)