From 52af6a65c52618d5e4707e81f9883b5066b6438b Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Mon, 25 Apr 2022 16:58:46 -0700 Subject: [PATCH] Don't flatten fragment spreads with differing directives Reviewed By: tyao1 Differential Revision: D35797243 fbshipit-source-id: 0c7e19e6d543b18556263bff8898dc583e62ee10 --- .../crates/graphql-ir/src/node_identifier.rs | 5 + ...fferent-field-args-are-not-merged.expected | 189 ++++++++++++++++++ ...ifferent-field-args-are-not-merged.graphql | 18 ++ .../tests/compile_relay_artifacts_test.rs | 9 +- compiler/crates/relay-transforms/src/util.rs | 3 +- 5 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected create mode 100644 compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.graphql diff --git a/compiler/crates/graphql-ir/src/node_identifier.rs b/compiler/crates/graphql-ir/src/node_identifier.rs index 02fa77d30faf8..ef216d295ab14 100644 --- a/compiler/crates/graphql-ir/src/node_identifier.rs +++ b/compiler/crates/graphql-ir/src/node_identifier.rs @@ -71,6 +71,8 @@ impl NodeIdentifier { (Selection::FragmentSpread(a), Selection::FragmentSpread(b)) => { a.fragment.item == b.fragment.item && a.arguments.location_agnostic_eq::(&b.arguments) + && a.directives + .location_agnostic_eq::(&b.directives) } (Selection::InlineFragment(a), Selection::InlineFragment(b)) => { a.type_condition == b.type_condition @@ -109,6 +111,8 @@ impl PartialEq for NodeIdentifier { l.fragment.item == r.fragment.item && l.arguments.location_agnostic_eq::(&r.arguments) + && l.directives + .location_agnostic_eq::(&r.directives) } (NodeIdentifierInner::InlineFragment(l), NodeIdentifierInner::InlineFragment(r)) => { l.type_condition == r.type_condition @@ -138,6 +142,7 @@ impl Hash for NodeIdentifier { 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/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.expected new file mode 100644 index 0000000000000..160b20a968799 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/compile_relay_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/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.graphql b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolvers-with-different-field-args-are-not-merged.graphql new file mode 100644 index 0000000000000..e87ff2587d9a3 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/compile_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 index 7472a54199599..2b4d3c5e8e935 100644 --- 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<> + * @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 index 1afe936b8a346..0c10373516f6a 100644 --- 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] = [