From 5d22d1c21031662368f35230fa580fe354b8325e Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Fri, 1 Sep 2023 15:27:41 -0700 Subject: [PATCH] Show a helpful error if a resolver returns an interface with no implementors (#4428) Summary: **Context:** Without this we generate a broken Flow type. We will eventually support Relay Resolvers that return interfaces. Today they are not permitted in the common case. However, once we do allow them, we must ensure that the schema includes at least once concrete type that implements that interface. Without that property, we will generate invalid Flow types. **This change** Adds additional validation for implementors of client-defined interfaces if they are being used as a return type for RelayResolvers and tests. Also updates an existing test to have it only test the existing validation for blocking interfaces as a return type. Pull Request resolved: https://github.com/facebook/relay/pull/4428 Test Plan: `./scripts/update-fixtures.sh` `cargo test` specifically: `cargo test -p relay-transforms --test client_edges_test` `cargo test -p relay-compiler --test relay_compiler_compile_relay_artifacts_test relay_resolver_edge_to_interface_with_no_implementors` `cargo test -p relay-compiler --test relay_compiler_compile_relay_artifacts_test relay_resolver_edge_to_interface_with_child_interface_and_no_implementors` Reviewed By: captbaritone Differential Revision: D48852264 Pulled By: monicatang fbshipit-source-id: 5d9ebbee4423eef8f1aaf4dd031e3e3227675f19 --- ...ild-interface-and-no-implementors.expected | 43 +++++++++++++++++++ ...hild-interface-and-no-implementors.graphql | 25 +++++++++++ ...to-interface-with-no-implementors.expected | 38 ++++++++++++++++ ...-to-interface-with-no-implementors.graphql | 20 +++++++++ .../tests/compile_relay_artifacts_test.rs | 16 ++++++- .../relay-transforms/src/client_edges.rs | 13 +++++- .../crates/relay-transforms/src/errors.rs | 2 +- ...-edge-to-client-interface.invalid.expected | 5 +++ ...t-edge-to-client-interface.invalid.graphql | 5 +++ ...h-unimplemented-interface.invalid.expected | 6 +-- 10 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.expected create mode 100644 compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql create mode 100644 compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.expected create mode 100644 compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.graphql diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.expected b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.expected new file mode 100644 index 0000000000000..71cb70653bb5b --- /dev/null +++ b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.expected @@ -0,0 +1,43 @@ +==================================== INPUT ==================================== +# expected-to-throw +query relayResolverEdgeToInterfaceWithChildInterfaceAndNoImplementorsQuery { + resolver_field { + name + } +} + +# %extensions% + +""" +An interface with no concrete implementors +""" +interface SomeInterface { + name: String +} + +interface ChildInterface implements SomeInterface { + name: String + age: Int +} + +extend type Query { + resolver_field: SomeInterface + @relay_resolver(import_path: "./path/to/Resolver.js") +} +==================================== ERROR ==================================== +✖︎ Client Edges that reference client-defined interface types are not currently supported in Relay. + + relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql:3:3 + 2 │ query relayResolverEdgeToInterfaceWithChildInterfaceAndNoImplementorsQuery { + 3 │ resolver_field { + │ ^^^^^^^^^^^^^^ + 4 │ name + + +✖︎ No types implement the client interface SomeInterface. Interfaces returned by a @RelayResolver must have at least one concrete implementation. + + :2:44 + 1 │ # expected-to-throw + 2 │ query relayResolverEdgeToInterfaceWithChildInterfaceAndNoImplementorsQuery { + │ ^^^^^^^^^^^^^ + 3 │ resolver_field { diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql new file mode 100644 index 0000000000000..996b9886d6989 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql @@ -0,0 +1,25 @@ +# expected-to-throw +query relayResolverEdgeToInterfaceWithChildInterfaceAndNoImplementorsQuery { + resolver_field { + name + } +} + +# %extensions% + +""" +An interface with no concrete implementors +""" +interface SomeInterface { + name: String +} + +interface ChildInterface implements SomeInterface { + name: String + age: Int +} + +extend type Query { + resolver_field: SomeInterface + @relay_resolver(import_path: "./path/to/Resolver.js") +} diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.expected b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.expected new file mode 100644 index 0000000000000..52481f5e54510 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.expected @@ -0,0 +1,38 @@ +==================================== INPUT ==================================== +# expected-to-throw +query relayResolverEdgeToInterfaceWithNoImplementorsQuery { + resolver_field { + name + } +} + +# %extensions% + +""" +An interface with no implementors +""" +interface SomeInterface { + name: String +} + +extend type Query { + resolver_field: SomeInterface + @relay_resolver(import_path: "./path/to/Resolver.js") +} +==================================== ERROR ==================================== +✖︎ Client Edges that reference client-defined interface types are not currently supported in Relay. + + relay-resolver-edge-to-interface-with-no-implementors.graphql:3:3 + 2 │ query relayResolverEdgeToInterfaceWithNoImplementorsQuery { + 3 │ resolver_field { + │ ^^^^^^^^^^^^^^ + 4 │ name + + +✖︎ No types implement the client interface SomeInterface. Interfaces returned by a @RelayResolver must have at least one concrete implementation. + + :2:35 + 1 │ # expected-to-throw + 2 │ query relayResolverEdgeToInterfaceWithNoImplementorsQuery { + │ ^^^^^^^^^^^^^ + 3 │ resolver_field { diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.graphql b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.graphql new file mode 100644 index 0000000000000..358021deb4e42 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.graphql @@ -0,0 +1,20 @@ +# expected-to-throw +query relayResolverEdgeToInterfaceWithNoImplementorsQuery { + resolver_field { + name + } +} + +# %extensions% + +""" +An interface with no implementors +""" +interface SomeInterface { + name: String +} + +extend type Query { + resolver_field: SomeInterface + @relay_resolver(import_path: "./path/to/Resolver.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 6669a2d726067..df989150aae72 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<<4c83edb5a97709b48ab136e569a1bc3e>> + * @generated SignedSource<> */ mod compile_relay_artifacts; @@ -1104,6 +1104,20 @@ fn relay_resolver_backing_client_edge() { test_fixture(transform_fixture, "relay-resolver-backing-client-edge.graphql", "compile_relay_artifacts/fixtures/relay-resolver-backing-client-edge.expected", input, expected); } +#[test] +fn relay_resolver_edge_to_interface_with_child_interface_and_no_implementors() { + let input = include_str!("compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql"); + let expected = include_str!("compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.expected"); + test_fixture(transform_fixture, "relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.graphql", "compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-child-interface-and-no-implementors.expected", input, expected); +} + +#[test] +fn relay_resolver_edge_to_interface_with_no_implementors() { + let input = include_str!("compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.graphql"); + let expected = include_str!("compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.expected"); + test_fixture(transform_fixture, "relay-resolver-edge-to-interface-with-no-implementors.graphql", "compile_relay_artifacts/fixtures/relay-resolver-edge-to-interface-with-no-implementors.expected", input, expected); +} + #[test] fn relay_resolver_es_modules() { let input = include_str!("compile_relay_artifacts/fixtures/relay-resolver-es-modules.graphql"); diff --git a/compiler/crates/relay-transforms/src/client_edges.rs b/compiler/crates/relay-transforms/src/client_edges.rs index a38759dfc9be4..1c4682c8ada56 100644 --- a/compiler/crates/relay-transforms/src/client_edges.rs +++ b/compiler/crates/relay-transforms/src/client_edges.rs @@ -363,7 +363,18 @@ impl<'program, 'sc> ClientEdgesTransform<'program, 'sc> { } match edge_to_type { - Type::Interface(_) => { + Type::Interface(interface_id) => { + let interface = schema.interface(interface_id); + let implementing_objects = + interface.recursively_implementing_objects(Arc::as_ref(schema)); + if implementing_objects.is_empty() { + self.errors.push(Diagnostic::error( + ValidationMessage::RelayResolverClientInterfaceMustBeImplemented { + interface_name: interface.name.item, + }, + interface.name.location, + )); + } if !has_output_type(resolver_directive) { self.errors.push(Diagnostic::error( ValidationMessage::ClientEdgeToClientInterface, diff --git a/compiler/crates/relay-transforms/src/errors.rs b/compiler/crates/relay-transforms/src/errors.rs index c99cb30c6ebf1..f93f1105094ee 100644 --- a/compiler/crates/relay-transforms/src/errors.rs +++ b/compiler/crates/relay-transforms/src/errors.rs @@ -168,7 +168,7 @@ pub enum ValidationMessage { }, #[error( - "No types implement the client interface {interface_name}. For a client interface to be used as a @RelayResolver @outputType, at least one Object type must implement the interface." + "No types implement the client interface {interface_name}. Interfaces returned by a @RelayResolver must have at least one concrete implementation." )] RelayResolverClientInterfaceMustBeImplemented { interface_name: InterfaceName }, diff --git a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected index d7b9a7bb61baa..0507ac5f45373 100644 --- a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected +++ b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected @@ -16,6 +16,11 @@ interface ClientOnlyInterface implements Node { id: ID! } +# Add a concrete type so that we don't trigger an unrelated compiler error. +type BestFriend implements ClientOnlyInterface { + id: ID! +} + extend type User { best_friend: ClientOnlyInterface @relay_resolver(fragment_name: "BestFriendResolverFragment_name", import_path: "BestFriendResolver") } diff --git a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql index 46c9dc032e4b3..9a3073baefde6 100644 --- a/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql +++ b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql @@ -15,6 +15,11 @@ interface ClientOnlyInterface implements Node { id: ID! } +# Add a concrete type so that we don't trigger an unrelated compiler error. +type BestFriend implements ClientOnlyInterface { + id: ID! +} + extend type User { best_friend: ClientOnlyInterface @relay_resolver(fragment_name: "BestFriendResolverFragment_name", import_path: "BestFriendResolver") } diff --git a/compiler/crates/relay-transforms/tests/generate_relay_resolvers_operations_for_nested_objects/fixtures/output-type-with-unimplemented-interface.invalid.expected b/compiler/crates/relay-transforms/tests/generate_relay_resolvers_operations_for_nested_objects/fixtures/output-type-with-unimplemented-interface.invalid.expected index 7cc440ddf2c50..c2801529553f4 100644 --- a/compiler/crates/relay-transforms/tests/generate_relay_resolvers_operations_for_nested_objects/fixtures/output-type-with-unimplemented-interface.invalid.expected +++ b/compiler/crates/relay-transforms/tests/generate_relay_resolvers_operations_for_nested_objects/fixtures/output-type-with-unimplemented-interface.invalid.expected @@ -32,7 +32,7 @@ extend type User { top_level: IStageName @relay_resolver(fragment_name: "PopStarNameResolverFragment_name", import_path: "PopStarNameResolver", has_output_type: true) } ==================================== ERROR ==================================== -✖︎ No types implement the client interface IStageName. For a client interface to be used as a @RelayResolver @outputType, at least one Object type must implement the interface. +✖︎ No types implement the client interface IStageName. Interfaces returned by a @RelayResolver must have at least one concrete implementation. output-type-with-unimplemented-interface.invalid.graphql:7:11 6 │ @@ -41,7 +41,7 @@ extend type User { 8 │ value: String -✖︎ No types implement the client interface IStageName. For a client interface to be used as a @RelayResolver @outputType, at least one Object type must implement the interface. +✖︎ No types implement the client interface IStageName. Interfaces returned by a @RelayResolver must have at least one concrete implementation. output-type-with-unimplemented-interface.invalid.graphql:7:11 6 │ @@ -50,7 +50,7 @@ extend type User { 8 │ value: String -✖︎ No types implement the client interface IStageName. For a client interface to be used as a @RelayResolver @outputType, at least one Object type must implement the interface. +✖︎ No types implement the client interface IStageName. Interfaces returned by a @RelayResolver must have at least one concrete implementation. output-type-with-unimplemented-interface.invalid.graphql:7:11 6 │