From bf6123f54313be1835a6f49130f75223e9768f36 Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Wed, 30 Aug 2023 13:16:42 -0700 Subject: [PATCH 1/6] [relay] Show a helpful error if a resolver returns an interface with no implementors --- ...to-interface-with-no-implementors.expected | 38 +++++++++++++++++++ ...-to-interface-with-no-implementors.graphql | 20 ++++++++++ .../tests/compile_relay_artifacts_test.rs | 9 ++++- .../relay-transforms/src/client_edges.rs | 13 ++++++- 4 files changed, 78 insertions(+), 2 deletions(-) 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-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..586fde7918234 --- /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. For a client interface to be used as a @RelayResolver @outputType, at least one Object type must implement the interface. + + :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..efcc110009d13 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<<27fd5fb3f07346c7af49129d773317a8>> */ mod compile_relay_artifacts; @@ -1104,6 +1104,13 @@ 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_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, From 0e07c43e3d71a24eb6ef68a70688aba24cbb858d Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Wed, 30 Aug 2023 13:36:59 -0700 Subject: [PATCH 2/6] Update client-edge-to-client-interface test --- .../fixtures/client-edge-to-client-interface.invalid.expected | 4 ++++ .../fixtures/client-edge-to-client-interface.invalid.graphql | 4 ++++ 2 files changed, 8 insertions(+) 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..4a8f207198905 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,10 @@ interface ClientOnlyInterface implements Node { id: ID! } +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..563113632789b 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,10 @@ interface ClientOnlyInterface implements Node { id: ID! } +type BestFriend implements ClientOnlyInterface { + id: ID! +} + extend type User { best_friend: ClientOnlyInterface @relay_resolver(fragment_name: "BestFriendResolverFragment_name", import_path: "BestFriendResolver") } From 74bb8abd92d13460fb90be290c65719cba74c62e Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Wed, 30 Aug 2023 15:08:55 -0700 Subject: [PATCH 3/6] address suggestions --- ...ild-interface-and-no-implementors.expected | 43 +++++++++++++++++++ ...hild-interface-and-no-implementors.graphql | 25 +++++++++++ ...to-interface-with-no-implementors.expected | 2 +- .../tests/compile_relay_artifacts_test.rs | 9 +++- .../crates/relay-transforms/src/errors.rs | 2 +- ...t-edge-to-client-interface.invalid.graphql | 1 + 6 files changed, 79 insertions(+), 3 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 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..30a755f375041 --- /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 @outputType 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 index 586fde7918234..81344417cc5a2 100644 --- 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 @@ -29,7 +29,7 @@ extend type Query { 4 │ name -✖︎ No types implement the client interface SomeInterface. 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 SomeInterface. Interfaces returned by a @RelayResolver @outputType must have at least one concrete implementation. :2:35 1 │ # expected-to-throw 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 efcc110009d13..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<<27fd5fb3f07346c7af49129d773317a8>> + * @generated SignedSource<> */ mod compile_relay_artifacts; @@ -1104,6 +1104,13 @@ 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"); diff --git a/compiler/crates/relay-transforms/src/errors.rs b/compiler/crates/relay-transforms/src/errors.rs index c99cb30c6ebf1..213efb912c51c 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 @outputType 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.graphql b/compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql index 563113632789b..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,7 @@ 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! } From a86d3e933a3616548ef79cf69635a1846d5c25ea Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Wed, 30 Aug 2023 16:35:44 -0700 Subject: [PATCH 4/6] delete @outputType from error msg --- compiler/crates/relay-transforms/src/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/crates/relay-transforms/src/errors.rs b/compiler/crates/relay-transforms/src/errors.rs index 213efb912c51c..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}. Interfaces returned by a @RelayResolver @outputType must have at least one concrete implementation." + "No types implement the client interface {interface_name}. Interfaces returned by a @RelayResolver must have at least one concrete implementation." )] RelayResolverClientInterfaceMustBeImplemented { interface_name: InterfaceName }, From f9f7f1376fa6d520ce5ce348dbc4a65b50028ad7 Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Wed, 30 Aug 2023 21:42:34 -0700 Subject: [PATCH 5/6] fix tests --- ...-interface-with-child-interface-and-no-implementors.expected | 2 +- ...lay-resolver-edge-to-interface-with-no-implementors.expected | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index 30a755f375041..71cb70653bb5b 100644 --- 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 @@ -34,7 +34,7 @@ extend type Query { 4 │ name -✖︎ No types implement the client interface SomeInterface. Interfaces returned by a @RelayResolver @outputType must have at least one concrete implementation. +✖︎ 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 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 index 81344417cc5a2..52481f5e54510 100644 --- 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 @@ -29,7 +29,7 @@ extend type Query { 4 │ name -✖︎ No types implement the client interface SomeInterface. Interfaces returned by a @RelayResolver @outputType must have at least one concrete implementation. +✖︎ 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 From d13a43cf9e704a6bcf4f1a3a6b85686272945f3e Mon Sep 17 00:00:00 2001 From: Monica Limin Tang Date: Thu, 31 Aug 2023 09:06:16 -0700 Subject: [PATCH 6/6] cargo test --- .../fixtures/client-edge-to-client-interface.invalid.expected | 1 + 1 file changed, 1 insertion(+) 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 4a8f207198905..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,7 @@ 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! }