Skip to content

Commit

Permalink
Allow configurable @defer/@stream names (#4467)
Browse files Browse the repository at this point in the history
Summary:
This is not an attempt to get Relay to conform to the current defer/stream spec draft, but rather just a small step in that direction by allowing configurable directive & argument names.

Currently Relay requires the argument `initial_count` on `stream`, but the GraphQL spec uses camelCase for all names.

This PR updates Relay compiler to follow the same pattern used by `ConnectionInterface`, introducing `DeferStreamInterface` to allow configuring the names and arguments of the defer & stream directive.

The first commit adds the configuration updating all touch points to reference the configuration.

The second commit changes the defaults to camelCase, to match the defaults used in `ConnectionInterface`. This makes the diff substantially larger as many tests and fixtures need to be updated. I'm happy to remove this commit from this PR if desired.

Pull Request resolved: #4467

Reviewed By: alunyov

Differential Revision: D50043794

Pulled By: captbaritone

fbshipit-source-id: e27f159ef8e22dd197b85e2653b9541c66594df1
  • Loading branch information
robrichard authored and facebook-github-bot committed Oct 18, 2023
1 parent 392ea1f commit 58da806
Show file tree
Hide file tree
Showing 87 changed files with 533 additions and 276 deletions.
13 changes: 12 additions & 1 deletion compiler/crates/common/src/named_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,18 @@ impl FromStr for DirectiveName {
}

impl_lookup!(DirectiveName);
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[derive(
Clone,
Copy,
Debug,
Deserialize,
Eq,
PartialEq,
Ord,
PartialOrd,
Hash,
Serialize
)]
pub struct ArgumentName(pub StringKey);

impl fmt::Display for ArgumentName {
Expand Down
39 changes: 30 additions & 9 deletions compiler/crates/relay-codegen/src/build_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ use relay_transforms::RequiredMetadataDirective;
use relay_transforms::ResolverOutputTypeInfo;
use relay_transforms::StreamDirective;
use relay_transforms::CLIENT_EXTENSION_DIRECTIVE_NAME;
use relay_transforms::DEFER_STREAM_CONSTANTS;
use relay_transforms::DIRECTIVE_SPLIT_OPERATION;
use relay_transforms::INLINE_DIRECTIVE_NAME;
use relay_transforms::INTERNAL_METADATA_DIRECTIVE;
Expand Down Expand Up @@ -482,7 +481,12 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
};
if metadata.is_stream_connection {
object.push(ObjectEntry {
key: DEFER_STREAM_CONSTANTS.stream_name.0,
key: self
.project_config
.schema_config
.defer_stream_interface
.stream_name
.0,
value: Primitive::Bool(true),
})
}
Expand Down Expand Up @@ -528,9 +532,12 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
vec![self.build_fragment_spread(frag_spread)]
}
Selection::InlineFragment(inline_fragment) => {
let defer = inline_fragment
.directives
.named(DEFER_STREAM_CONSTANTS.defer_name);
let defer = inline_fragment.directives.named(
self.project_config
.schema_config
.defer_stream_interface
.defer_name,
);
if let Some(defer) = defer {
vec![self.build_defer(context, inline_fragment, defer)]
} else if let Some(inline_data_directive) =
Expand Down Expand Up @@ -577,7 +584,12 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
}
}
Selection::LinkedField(field) => {
let stream = field.directives.named(DEFER_STREAM_CONSTANTS.stream_name);
let stream = field.directives.named(
self.project_config
.schema_config
.defer_stream_interface
.stream_name,
);

match stream {
Some(stream) => vec![self.build_stream(context, field, stream)],
Expand Down Expand Up @@ -1254,7 +1266,10 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
defer: &Directive,
) -> Primitive {
let next_selections = self.build_selections(context, inline_fragment.selections.iter());
let DeferDirective { if_arg, label_arg } = DeferDirective::from(defer);
let DeferDirective { if_arg, label_arg } = DeferDirective::from(
defer,
&self.project_config.schema_config.defer_stream_interface,
);
let if_variable_name = if_arg.and_then(|arg| match &arg.value.item {
// `true` is the default, remove as the AST is typed just as a variable name string
// `false` constant values should've been transformed away in skip_unreachable_node
Expand Down Expand Up @@ -1283,7 +1298,10 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
&LinkedField {
directives: remove_directive(
&linked_field.directives,
DEFER_STREAM_CONSTANTS.stream_name,
self.project_config
.schema_config
.defer_stream_interface
.stream_name,
),
..linked_field.to_owned()
},
Expand All @@ -1300,7 +1318,10 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
label_arg,
use_customized_batch_arg: _,
initial_count_arg: _,
} = StreamDirective::from(stream);
} = StreamDirective::from(
stream,
&self.project_config.schema_config.defer_stream_interface,
);
let if_variable_name = if_arg.and_then(|arg| match &arg.value.item {
// `true` is the default, remove as the AST is typed just as a variable name string
// `false` constant values should've been transformed away in skip_unreachable_node
Expand Down
5 changes: 4 additions & 1 deletion compiler/crates/relay-codegen/tests/connections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use graphql_test_helpers::diagnostics_to_sorted_string;
use relay_codegen::build_request_params;
use relay_codegen::JsModuleFormat;
use relay_codegen::Printer;
use relay_config::DeferStreamInterface;
use relay_config::ProjectConfig;
use relay_test_schema::get_test_schema;
use relay_transforms::transform_connections;
Expand All @@ -41,11 +42,13 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
let program = Program::from_definitions(Arc::clone(&schema), ir);

let connection_interface = ConnectionInterface::default();
let defer_stream_interface = DeferStreamInterface::default();

validate_connections(&program, &connection_interface)
.map_err(|diagnostics| diagnostics_to_sorted_string(fixture.content, &diagnostics))?;

let next_program = transform_connections(&program, &connection_interface);
let next_program =
transform_connections(&program, &connection_interface, &defer_stream_interface);

let mut printed = next_program
.operations()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ query QueryWithFragmentWithStream($id: ID!) {

fragment FeedbackFragment on Feedback {
id
actors @stream(initial_count: 1) {
actors @stream(initialCount: 1) {
name
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ query QueryWithFragmentWithStream($id: ID!) {

fragment FeedbackFragment on Feedback {
id
actors @stream(initial_count: 1) {
actors @stream(initialCount: 1) {
name
}
}
5 changes: 4 additions & 1 deletion compiler/crates/relay-codegen/tests/defer_stream/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use graphql_syntax::parse_executable;
use relay_codegen::print_fragment;
use relay_codegen::print_operation;
use relay_codegen::JsModuleFormat;
use relay_config::DeferStreamInterface;
use relay_config::ProjectConfig;
use relay_test_schema::get_test_schema;
use relay_transforms::sort_selections;
Expand All @@ -29,7 +30,9 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
let schema = get_test_schema();
let ir = build(&schema, &ast.definitions).unwrap();
let program = Program::from_definitions(Arc::clone(&schema), ir);
let next_program = sort_selections(&transform_defer_stream(&program).unwrap());
let defer_stream_interface = DeferStreamInterface::default();
let next_program =
sort_selections(&transform_defer_stream(&program, &defer_stream_interface).unwrap());
let mut result = next_program
.fragments()
.map(|def| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ query fragmentWithDeferInStream_QueryWithFragmentWithStreamQuery($id: ID!) {

fragment fragmentWithDeferInStream_FeedbackFragment on Feedback {
id
actors @stream(initial_count: 1, label: "StreamedActorsLabel") {
actors @stream(initialCount: 1, label: "StreamedActorsLabel") {
...fragmentWithDeferInStream_ActorFragment @defer
}
}
Expand Down Expand Up @@ -194,7 +194,7 @@ fragment fragmentWithDeferInStream_ActorFragment on Actor {

fragment fragmentWithDeferInStream_FeedbackFragment on Feedback {
id
actors @stream(label: "fragmentWithDeferInStream_FeedbackFragment$stream$StreamedActorsLabel", initial_count: 1) {
actors @stream(label: "fragmentWithDeferInStream_FeedbackFragment$stream$StreamedActorsLabel", initialCount: 1) {
__typename
...fragmentWithDeferInStream_ActorFragment @defer(label: "fragmentWithDeferInStream_FeedbackFragment$defer$fragmentWithDeferInStream_ActorFragment")
id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ query fragmentWithDeferInStream_QueryWithFragmentWithStreamQuery($id: ID!) {

fragment fragmentWithDeferInStream_FeedbackFragment on Feedback {
id
actors @stream(initial_count: 1, label: "StreamedActorsLabel") {
actors @stream(initialCount: 1, label: "StreamedActorsLabel") {
...fragmentWithDeferInStream_ActorFragment @defer
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ query fragmentWithStream_QueryWithFragmentWithStreamQuery($id: ID!) {

fragment fragmentWithStream_FeedbackFragment on Feedback {
id
actors @stream(initial_count: 1, label: "StreamedActorsLabel") {
actors @stream(initialCount: 1, label: "StreamedActorsLabel") {
name
}
}
Expand Down Expand Up @@ -174,7 +174,7 @@ query fragmentWithStream_QueryWithFragmentWithStreamQuery(

fragment fragmentWithStream_FeedbackFragment on Feedback {
id
actors @stream(label: "fragmentWithStream_FeedbackFragment$stream$StreamedActorsLabel", initial_count: 1) {
actors @stream(label: "fragmentWithStream_FeedbackFragment$stream$StreamedActorsLabel", initialCount: 1) {
__typename
name
id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ query fragmentWithStream_QueryWithFragmentWithStreamQuery($id: ID!) {

fragment fragmentWithStream_FeedbackFragment on Feedback {
id
actors @stream(initial_count: 1, label: "StreamedActorsLabel") {
actors @stream(initialCount: 1, label: "StreamedActorsLabel") {
name
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ fragment refetchableFragmentWithConnectionWithStream_PaginationFragment_1G22uz o
... on User {
name
friends(after: $cursor, first: $count) {
edges @stream(label: "refetchableFragmentWithConnectionWithStream_PaginationFragment$stream$PaginationFragment_friends", initial_count: 1) {
edges @stream(label: "refetchableFragmentWithConnectionWithStream_PaginationFragment$stream$PaginationFragment_friends", initialCount: 1) {
node {
id
__typename
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ query selectionSetConflictInconsistentStreamUsage1Query {
... on User {
friends {
... on FriendsConnection {
edges @stream(label: "hdijf", initial_count: 1) {
edges @stream(label: "hdijf", initialCount: 1) {
node {
name
}
Expand All @@ -25,7 +25,7 @@ query selectionSetConflictInconsistentStreamUsage1Query {

selection_set_conflict_inconsistent_stream_usage_1.graphql:7:11
6 │ ... on FriendsConnection {
7 │ edges @stream(label: "hdijf", initial_count: 1) {
7 │ edges @stream(label: "hdijf", initialCount: 1) {
│ ^^^^^
8 │ node {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ query selectionSetConflictInconsistentStreamUsage1Query {
... on User {
friends {
... on FriendsConnection {
edges @stream(label: "hdijf", initial_count: 1) {
edges @stream(label: "hdijf", initialCount: 1) {
node {
name
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ query selectionSetConflictInconsistentStreamUsage2Query {
... on User {
friends {
... on FriendsConnection {
edges @stream(label: "hdijf", initial_count: 1) {
edges @stream(label: "hdijf", initialCount: 1) {
node {
name
}
}
}
edges @stream(label: "hkjdf", initial_count: 2) {
edges @stream(label: "hkjdf", initialCount: 2) {
node {
id
}
Expand All @@ -25,14 +25,14 @@ query selectionSetConflictInconsistentStreamUsage2Query {

selection_set_conflict_inconsistent_stream_usage_2.graphql:7:11
6 │ ... on FriendsConnection {
7 │ edges @stream(label: "hdijf", initial_count: 1) {
7 │ edges @stream(label: "hdijf", initialCount: 1) {
│ ^^^^^
8 │ node {

ℹ︎ the other field

selection_set_conflict_inconsistent_stream_usage_2.graphql:13:9
12 │ }
13 │ edges @stream(label: "hkjdf", initial_count: 2) {
13 │ edges @stream(label: "hkjdf", initialCount: 2) {
│ ^^^^^
14 │ node {
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ query selectionSetConflictInconsistentStreamUsage2Query {
... on User {
friends {
... on FriendsConnection {
edges @stream(label: "hdijf", initial_count: 1) {
edges @stream(label: "hdijf", initialCount: 1) {
node {
name
}
}
}
edges @stream(label: "hkjdf", initial_count: 2) {
edges @stream(label: "hkjdf", initialCount: 2) {
node {
id
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ query selectionSetConflictStreamOnNodesOrEdgesQuery {
me {
... on User {
friends {
edges @stream(label: "b", initial_count: 1) {
edges @stream(label: "b", initialCount: 1) {
node {
id
}
Expand Down Expand Up @@ -168,7 +168,7 @@ QUERY:
query selectionSetConflictStreamOnNodesOrEdgesQuery {
me {
friends {
edges @stream(label: "selectionSetConflictStreamOnNodesOrEdgesQuery$stream$b", initial_count: 1) {
edges @stream(label: "selectionSetConflictStreamOnNodesOrEdgesQuery$stream$b", initialCount: 1) {
node {
id
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ query selectionSetConflictStreamOnNodesOrEdgesQuery {
me {
... on User {
friends {
edges @stream(label: "b", initial_count: 1) {
edges @stream(label: "b", initialCount: 1) {
node {
id
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ query selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoQuery {
hasNextPage
}
}
edges @stream(label: "b", initial_count: 1) {
edges @stream(label: "b", initialCount: 1) {
node {
id
}
Expand Down Expand Up @@ -213,7 +213,7 @@ query selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoQuery {
pageInfo {
hasNextPage
}
edges @stream(label: "selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoQuery$stream$b", initial_count: 1) {
edges @stream(label: "selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoQuery$stream$b", initialCount: 1) {
node {
id
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ query selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoQuery {
hasNextPage
}
}
edges @stream(label: "b", initial_count: 1) {
edges @stream(label: "b", initialCount: 1) {
node {
id
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ query selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoAndPageInfoA
hasNextPage
}
}
edges @stream(label: "b", initial_count: 1) {
edges @stream(label: "b", initialCount: 1) {
node {
id
}
Expand Down Expand Up @@ -213,7 +213,7 @@ query selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoAndPageInfoA
pagination: pageInfo {
hasNextPage
}
edges @stream(label: "selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoAndPageInfoAliasQuery$stream$b", initial_count: 1) {
edges @stream(label: "selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoAndPageInfoAliasQuery$stream$b", initialCount: 1) {
node {
id
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ query selectionSetConflictStreamOnNodesOrEdgesWithoutDeferOnPageInfoAndPageInfoA
hasNextPage
}
}
edges @stream(label: "b", initial_count: 1) {
edges @stream(label: "b", initialCount: 1) {
node {
id
}
Expand Down
Loading

0 comments on commit 58da806

Please sign in to comment.