Skip to content

Commit

Permalink
Show a helpful error if a resolver returns an interface with no imple…
Browse files Browse the repository at this point in the history
…mentors (#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: #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
  • Loading branch information
Monica Limin Tang authored and facebook-github-bot committed Sep 1, 2023
1 parent a3fbc12 commit 5d22d1c
Show file tree
Hide file tree
Showing 10 changed files with 167 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -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.

<generated>:2:44
1 │ # expected-to-throw
2 │ query relayResolverEdgeToInterfaceWithChildInterfaceAndNoImplementorsQuery {
│ ^^^^^^^^^^^^^
3 │ resolver_field {
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -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.

<generated>:2:35
1 │ # expected-to-throw
2 │ query relayResolverEdgeToInterfaceWithNoImplementorsQuery {
│ ^^^^^^^^^^^^^
3 │ resolver_field {
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<<c12ec82876dcc0420715ee575dae6f2f>>
*/

mod compile_relay_artifacts;
Expand Down Expand Up @@ -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");
Expand Down
13 changes: 12 additions & 1 deletion compiler/crates/relay-transforms/src/client_edges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/crates/relay-transforms/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 │
Expand All @@ -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 │
Expand All @@ -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 │
Expand Down

0 comments on commit 5d22d1c

Please sign in to comment.