Support @argumentDefinitions in @inline fragments (#3935)

Summary: This PR is the followup to #3933. Now we support both query & fragment variables inside of `inline` directives. Fixes #3106 ### What changed? - Added `args` to the `InlineDirectiveMetadata` struct - Piped those args through to the AST in `build_ast` - Updated `RelayReader` to temporarily swap out the variables while traversing an inline fragment Pull Request resolved: #3935 Reviewed By: alunyov Differential Revision: D36937215 Pulled By: captbaritone fbshipit-source-id: d680d8d47580fdc9ebc79c12811514b10ae97f72
facebook · Jun 23, 2022 · 90ccda1 · 90ccda1
1 parent da757ca
commit 90ccda1
Show file tree

Hide file tree

Showing 26 changed files with 1,328 additions and 120 deletions.
diff --git a/compiler/crates/graphql-ir/src/ir.rs b/compiler/crates/graphql-ir/src/ir.rs
@@ -80,7 +80,7 @@ pub struct FragmentDefinition {
 }
 
 /// A variable definition of an operation or fragment
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct VariableDefinition {
     pub name: WithLocation<StringKey>,
     pub type_: TypeReference,
@@ -326,7 +326,7 @@ impl Condition {
 // Associated Types
 
 /// @ Name Arguments?
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct Directive {
     pub name: WithLocation<StringKey>,
     pub arguments: Vec<Argument>,

diff --git a/compiler/crates/relay-codegen/src/build_ast.rs b/compiler/crates/relay-codegen/src/build_ast.rs
@@ -1452,10 +1452,21 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
         inline_directive_data: &InlineDirectiveMetadata,
     ) -> Primitive {
         let selections = self.build_selections(context, inline_fragment.selections.iter());
+        let args = self.build_arguments(&inline_directive_data.arguments);
+        let argument_definitions = self.build_fragment_variable_definitions(
+            &inline_directive_data.variable_definitions,
+            &inline_directive_data.used_global_variables,
+        );
+
         Primitive::Key(self.object(object! {
             kind: Primitive::String(CODEGEN_CONSTANTS.inline_data_fragment_spread),
             name: Primitive::String(inline_directive_data.fragment_name),
             selections: selections,
+            args: match args {
+                None => Primitive::SkippableNull,
+                Some(key) => Primitive::Key(key),
+            },
+            argument_definitions: argument_definitions,
         }))
     }
 

diff --git a/...compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment-global-vars.expected b/...compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment-global-vars.expected
@@ -53,6 +53,13 @@ fragment inlineDataFragmentGlobalVarsProfile on User @inline {
               ],
               "storageKey": null
             }
+          ],
+          "args": null,
+          "argumentDefinitions": [
+            {
+              "kind": "RootArgument",
+              "name": "pictureSize"
+            }
           ]
         }
       ],

diff --git a/...-compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment-local-args.expected b/...-compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment-local-args.expected
@@ -1,5 +1,4 @@
 ==================================== INPUT ====================================
-# expected-to-throw
 fragment inlineDataFragmentLocalArgsFragment on Query {
   usingLiteralPassedValue: me {
     ...inlineDataFragmentLocalArgsProfile @arguments(sizeArg: [100])
@@ -19,53 +18,191 @@ fragment inlineDataFragmentLocalArgsProfile on User
     uri
   }
 }
-==================================== ERROR ====================================
-✖︎ Variables from @argumentDefinitions are not yet supported inside @inline fragments.
-
-  inline-data-fragment-local-args.graphql:14:10
-   13 │ 
-   14 │ fragment inlineDataFragmentLocalArgsProfile on User
-      │          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   15 │   @inline
-
-  ℹ︎ Variable used:
-
-  inline-data-fragment-local-args.graphql:16:24
-   15 │   @inline
-   16 │   @argumentDefinitions(sizeArg: {type: "[Int]", defaultValue: [50]}) {
-      │                        ^^^^^^^
-   17 │   profilePicture(size: $sizeArg) {
-
-
-✖︎ Variables from @argumentDefinitions are not yet supported inside @inline fragments.
-
-  inline-data-fragment-local-args.graphql:14:10
-   13 │ 
-   14 │ fragment inlineDataFragmentLocalArgsProfile on User
-      │          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   15 │   @inline
-
-  ℹ︎ Variable used:
-
-  inline-data-fragment-local-args.graphql:16:24
-   15 │   @inline
-   16 │   @argumentDefinitions(sizeArg: {type: "[Int]", defaultValue: [50]}) {
-      │                        ^^^^^^^
-   17 │   profilePicture(size: $sizeArg) {
-
-
-✖︎ Variables from @argumentDefinitions are not yet supported inside @inline fragments.
-
-  inline-data-fragment-local-args.graphql:14:10
-   13 │ 
-   14 │ fragment inlineDataFragmentLocalArgsProfile on User
-      │          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   15 │   @inline
-
-  ℹ︎ Variable used:
+==================================== OUTPUT ===================================
+{
+  "argumentDefinitions": [
+    {
+      "kind": "RootArgument",
+      "name": "globalSizeVar"
+    }
+  ],
+  "kind": "Fragment",
+  "metadata": null,
+  "name": "inlineDataFragmentLocalArgsFragment",
+  "selections": [
+    {
+      "alias": "usingLiteralPassedValue",
+      "args": null,
+      "concreteType": "User",
+      "kind": "LinkedField",
+      "name": "me",
+      "plural": false,
+      "selections": [
+        {
+          "kind": "InlineDataFragmentSpread",
+          "name": "inlineDataFragmentLocalArgsProfile",
+          "selections": [
+            {
+              "alias": null,
+              "args": [
+                {
+                  "kind": "Variable",
+                  "name": "size",
+                  "variableName": "sizeArg"
+                }
+              ],
+              "concreteType": "Image",
+              "kind": "LinkedField",
+              "name": "profilePicture",
+              "plural": false,
+              "selections": [
+                {
+                  "alias": null,
+                  "args": null,
+                  "kind": "ScalarField",
+                  "name": "uri",
+                  "storageKey": null
+                }
+              ],
+              "storageKey": null
+            }
+          ],
+          "args": [
+            {
+              "kind": "Literal",
+              "name": "sizeArg",
+              "value": [
+                100
+              ]
+            }
+          ],
+          "argumentDefinitions": [
+            {
+              "defaultValue": [
+                50
+              ],
+              "kind": "LocalArgument",
+              "name": "sizeArg"
+            }
+          ]
+        }
+      ],
+      "storageKey": null
+    },
+    {
+      "alias": "usingGlobalPassedValue",
+      "args": null,
+      "concreteType": "User",
+      "kind": "LinkedField",
+      "name": "me",
+      "plural": false,
+      "selections": [
+        {
+          "kind": "InlineDataFragmentSpread",
+          "name": "inlineDataFragmentLocalArgsProfile",
+          "selections": [
+            {
+              "alias": null,
+              "args": [
+                {
+                  "kind": "Variable",
+                  "name": "size",
+                  "variableName": "sizeArg"
+                }
+              ],
+              "concreteType": "Image",
+              "kind": "LinkedField",
+              "name": "profilePicture",
+              "plural": false,
+              "selections": [
+                {
+                  "alias": null,
+                  "args": null,
+                  "kind": "ScalarField",
+                  "name": "uri",
+                  "storageKey": null
+                }
+              ],
+              "storageKey": null
+            }
+          ],
+          "args": [
+            {
+              "kind": "Variable",
+              "name": "sizeArg",
+              "variableName": "globalSizeVar"
+            }
+          ],
+          "argumentDefinitions": [
+            {
+              "defaultValue": [
+                50
+              ],
+              "kind": "LocalArgument",
+              "name": "sizeArg"
+            }
+          ]
+        }
+      ],
+      "storageKey": null
+    },
+    {
+      "alias": "usingDefaultValue",
+      "args": null,
+      "concreteType": "User",
+      "kind": "LinkedField",
+      "name": "me",
+      "plural": false,
+      "selections": [
+        {
+          "kind": "InlineDataFragmentSpread",
+          "name": "inlineDataFragmentLocalArgsProfile",
+          "selections": [
+            {
+              "alias": null,
+              "args": [
+                {
+                  "kind": "Variable",
+                  "name": "size",
+                  "variableName": "sizeArg"
+                }
+              ],
+              "concreteType": "Image",
+              "kind": "LinkedField",
+              "name": "profilePicture",
+              "plural": false,
+              "selections": [
+                {
+                  "alias": null,
+                  "args": null,
+                  "kind": "ScalarField",
+                  "name": "uri",
+                  "storageKey": null
+                }
+              ],
+              "storageKey": null
+            }
+          ],
+          "args": null,
+          "argumentDefinitions": [
+            {
+              "defaultValue": [
+                50
+              ],
+              "kind": "LocalArgument",
+              "name": "sizeArg"
+            }
+          ]
+        }
+      ],
+      "storageKey": null
+    }
+  ],
+  "type": "Query",
+  "abstractKey": null
+}
 
-  inline-data-fragment-local-args.graphql:16:24
-   15 │   @inline
-   16 │   @argumentDefinitions(sizeArg: {type: "[Int]", defaultValue: [50]}) {
-      │                        ^^^^^^^
-   17 │   profilePicture(size: $sizeArg) {
+{
+  "kind": "InlineDataFragment",
+  "name": "inlineDataFragmentLocalArgsProfile"
+}
diff --git a/...y-compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment-local-args.graphql b/...y-compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment-local-args.graphql
@@ -1,4 +1,3 @@
-# expected-to-throw
 fragment inlineDataFragmentLocalArgsFragment on Query {
   usingLiteralPassedValue: me {
     ...inlineDataFragmentLocalArgsProfile @arguments(sizeArg: [100])

diff --git a/...rates/relay-compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment.expected b/...rates/relay-compiler/tests/compile_relay_artifacts/fixtures/inline-data-fragment.expected
@@ -264,7 +264,9 @@ fragment inlineDataFragment_Profile on User {
               ],
               "storageKey": "profilePicture(size:100)"
             }
-          ]
+          ],
+          "args": null,
+          "argumentDefinitions": []
         },
         {
           "alias": null,
@@ -356,7 +358,9 @@ fragment inlineDataFragment_Profile on User {
               "type": "User",
               "abstractKey": null
             }
-          ]
+          ],
+          "args": null,
+          "argumentDefinitions": []
         }
       ],
       "storageKey": "username(name:\"test\")"

diff --git a/compiler/crates/relay-transforms/src/inline_data_fragment.rs b/compiler/crates/relay-transforms/src/inline_data_fragment.rs
@@ -7,8 +7,8 @@
 
 use common::{Diagnostic, DiagnosticsResult, Location, NamedItem, WithLocation};
 use graphql_ir::{
-    associated_data_impl, FragmentSpread, InlineFragment, Program, Selection, Transformed,
-    Transformer,
+    associated_data_impl, Argument, FragmentSpread, InlineFragment, Program, Selection,
+    Transformed, Transformer, VariableDefinition,
 };
 
 use intern::string_key::{Intern, StringKey};
@@ -50,6 +50,9 @@ impl<'s> InlineDataFragmentsTransform<'s> {
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct InlineDirectiveMetadata {
     pub fragment_name: StringKey,
+    pub arguments: Vec<Argument>,
+    pub variable_definitions: Vec<VariableDefinition>,
+    pub used_global_variables: Vec<VariableDefinition>,
 }
 associated_data_impl!(InlineDirectiveMetadata);
 
@@ -68,20 +71,6 @@ impl<'s> Transformer for InlineDataFragmentsTransform<'s> {
         if fragment.directives.named(*INLINE_DIRECTIVE_NAME).is_none() {
             next_fragment_spread
         } else {
-            if !fragment.variable_definitions.is_empty() {
-                let mut error = Diagnostic::error(
-                    ValidationMessage::InlineDataFragmentArgumentsNotSupported,
-                    fragment.name.location,
-                );
-                for var in fragment
-                    .variable_definitions
-                    .iter()
-                    .chain(fragment.used_global_variables.iter())
-                {
-                    error = error.annotate("Variable used:", var.name.location);
-                }
-                self.errors.push(error);
-            }
             match &next_fragment_spread {
                 Transformed::Keep => {
                     if !spread.directives.is_empty() {
@@ -147,6 +136,9 @@ impl<'s> Transformer for InlineDataFragmentsTransform<'s> {
                 directives: vec![
                     InlineDirectiveMetadata {
                         fragment_name: name,
+                        arguments: spread.arguments.clone(),
+                        variable_definitions: fragment.variable_definitions.clone(),
+                        used_global_variables: fragment.used_global_variables.clone(),
                     }
                     .into(),
                 ],
@@ -169,9 +161,6 @@ enum ValidationMessage {
     #[error("Found a circular reference from fragment '{fragment_name}'.")]
     CircularFragmentReference { fragment_name: StringKey },
 
-    #[error("Variables from @argumentDefinitions are not yet supported inside @inline fragments.")]
-    InlineDataFragmentArgumentsNotSupported,
-
     #[error("Directives on fragment spreads for @inline fragments are not yet supported")]
     InlineDataFragmentDirectivesNotSupported,
 }