Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support @argumentDefinitions in @inline fragments #3935

Closed
wants to merge 46 commits into from
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
7dc1772
remove error re: no query vars with @inline
tbezman Jun 1, 2022
e57d9b0
write basic test
tbezman Jun 1, 2022
315b946
generate relay artifacts
tbezman Jun 1, 2022
6d9c3fd
remove failing tests
tbezman Jun 1, 2022
c3730dd
test structure
tbezman Jun 1, 2022
e1dc692
update inline_data_fragment transform to support query variables
tbezman Jun 1, 2022
46921a0
update error message to better target argument definitions
tbezman Jun 1, 2022
f4c2a52
update compiler tests
tbezman Jun 1, 2022
104aa6c
Update transform test to test argument definitions instead
tbezman Jun 1, 2022
cdef5b7
remove variable usage check
tbezman Jun 1, 2022
a61cf53
pipe arguments through in inline directive metadata
tbezman Jun 1, 2022
e82304f
update compiler test
tbezman Jun 1, 2022
a7106c6
update InlineDataFragmentSpread to include the args type
tbezman Jun 1, 2022
aced514
update RelayReader to temporarily swap variables for the spread's tra…
tbezman Jun 1, 2022
e2aa075
add more tests
tbezman Jun 1, 2022
544a7e7
update generated artifacts
tbezman Jun 1, 2022
b3eef4d
update test snapshots
tbezman Jun 1, 2022
26bed58
Merge branch 'main' of https://github.com/facebook/relay into inline-…
tbezman Jun 1, 2022
05b57fc
Merge branch 'main' into inline-fragment-variables
tbezman Jun 2, 2022
ebb65ce
Merge remote-tracking branch 'upstream/main' into inline-fragment-var…
tbezman Jun 2, 2022
e6712f4
reset cargo lock
tbezman Jun 2, 2022
31ead11
rename vars
tbezman Jun 2, 2022
ddec4b3
Update packages/relay-runtime/store/__tests__/readInlineData-test.js
tbezman Jun 6, 2022
1fd27da
Update packages/relay-runtime/store/RelayReader.js
tbezman Jun 6, 2022
1998b16
update variable name
tbezman Jun 9, 2022
21be23c
Merge branch 'inline-fragment-variables' of github.com:tbezman/relay …
tbezman Jun 9, 2022
0ab0549
Merge remote-tracking branch 'upstream/main' into inline-fragment-var…
tbezman Jun 9, 2022
1ef590c
prettier
tbezman Jun 9, 2022
ce49c97
rust fmt
tbezman Jun 10, 2022
71db76e
add argument definitions to inline spread metadata
tbezman Jun 10, 2022
5dd113d
regen artifacts
tbezman Jun 10, 2022
4902432
update relay concrete variables to accept inline data fragment spreads
tbezman Jun 10, 2022
f6a2e81
update relay reader to use getFragmentVariables
tbezman Jun 10, 2022
f1b420b
add argument definitions to ReaderInlineDataFragmentSpread type
tbezman Jun 10, 2022
e4d63ab
update snapshots
tbezman Jun 10, 2022
62b114b
rustfmt
tbezman Jun 10, 2022
fc2310a
update more snapshots
tbezman Jun 10, 2022
c438692
Merge branch 'main' into inline-fragment-variables
tbezman Jun 16, 2022
3e3d907
Merge branch 'inline-fragment-variables' of github.com:tbezman/relay …
tbezman Jun 16, 2022
e2a3f19
create failing test case for nested fragments
tbezman Jun 16, 2022
aca0833
fix failing test case
tbezman Jun 16, 2022
c94ad3d
relay gen
tbezman Jun 16, 2022
14da635
add default variable to test
tbezman Jun 16, 2022
655fd17
relay gen
tbezman Jun 16, 2022
efdbece
gorgeous suggestion from Jordan
tbezman Jun 17, 2022
6a710d1
Merge branch 'main' into inline-fragment-variables
tbezman Jun 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler/crates/relay-codegen/src/build_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1418,10 +1418,16 @@ 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);

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),
},
}))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ fragment inlineDataFragmentGlobalVarsProfile on User @inline {
],
"storageKey": null
}
]
],
"args": null
}
],
"storageKey": null
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
==================================== INPUT ====================================
# expected-to-throw
fragment inlineDataFragmentLocalArgsFragment on Query {
usingLiteralPassedValue: me {
...inlineDataFragmentLocalArgsProfile @arguments(sizeArg: [100])
Expand All @@ -19,53 +18,164 @@ fragment inlineDataFragmentLocalArgsProfile on User
uri
}
}
==================================== ERROR ====================================
✖︎ Variables from @argumentDefinitions are not yet supported inside @inline fragments.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So exciting! Love seeing "not supported" tests turning green.


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
]
}
]
}
],
"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"
}
]
}
],
"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
}
],
"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"
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# expected-to-throw
fragment inlineDataFragmentLocalArgsFragment on Query {
usingLiteralPassedValue: me {
...inlineDataFragmentLocalArgsProfile @arguments(sizeArg: [100])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ fragment inlineDataFragment_Profile on User {
],
"storageKey": "profilePicture(size:100)"
}
]
],
"args": null
},
{
"alias": null,
Expand Down Expand Up @@ -355,7 +356,8 @@ fragment inlineDataFragment_Profile on User {
"type": "User",
"abstractKey": null
}
]
],
"args": null
}
],
"storageKey": "username(name:\"test\")"
Expand Down
33 changes: 8 additions & 25 deletions compiler/crates/relay-transforms/src/inline_data_fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

use intern::string_key::{Intern, StringKey};
Expand Down Expand Up @@ -50,6 +50,7 @@ impl<'s> InlineDataFragmentsTransform<'s> {
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct InlineDirectiveMetadata {
pub fragment_name: StringKey,
pub arguments: Vec<Argument>,
}
associated_data_impl!(InlineDirectiveMetadata);

Expand All @@ -68,20 +69,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() {
Expand Down Expand Up @@ -144,12 +131,11 @@ impl<'s> Transformer for InlineDataFragmentsTransform<'s> {

let inline_fragment = InlineFragment {
type_condition: None,
directives: vec![
InlineDirectiveMetadata {
fragment_name: name,
}
.into(),
],
directives: vec![InlineDirectiveMetadata {
fragment_name: name,
arguments: spread.arguments.clone(),
}
.into()],
selections: vec![Selection::InlineFragment(Arc::new(InlineFragment {
type_condition: Some(fragment.type_condition),
directives: vec![],
Expand All @@ -169,9 +155,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,
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ fragment InlineDataFragment on Image @inline {
... @__InlineDirectiveMetadata
# InlineDirectiveMetadata {
# fragment_name: "AnotherInlineDataFragment",
# arguments: [],
# }
{
... on Image {
Expand All @@ -43,13 +44,15 @@ fragment UserProfile on User {
... @__InlineDirectiveMetadata
# InlineDirectiveMetadata {
# fragment_name: "InlineDataFragment",
# arguments: [],
# }
{
... on Image {
uri
... @__InlineDirectiveMetadata
# InlineDirectiveMetadata {
# fragment_name: "AnotherInlineDataFragment",
# arguments: [],
# }
{
... on Image {
Expand All @@ -62,6 +65,7 @@ fragment UserProfile on User {
... @include(if: $cond) @__InlineDirectiveMetadata
# InlineDirectiveMetadata {
# fragment_name: "AnotherInlineDataFragment",
# arguments: [],
# }
{
... on Image {
Expand All @@ -74,13 +78,15 @@ fragment UserProfile on User {
... @__InlineDirectiveMetadata
# InlineDirectiveMetadata {
# fragment_name: "InlineDataFragment",
# arguments: [],
# }
{
... on Image {
uri
... @__InlineDirectiveMetadata
# InlineDirectiveMetadata {
# fragment_name: "AnotherInlineDataFragment",
# arguments: [],
# }
{
... on Image {
Expand Down
Loading