push validation on weak type/legacy syntax to before model resolver c…

…reation

Summary:
## Context

If the model that the client edge points to is a weak type or if the client edge is pointing to an object defined with legacy syntax, we skip creating model resolvers (for weak client edges, the model resolvers are unnecessary and for legacy edges, the model resolvers don't work properly since the generated id fragment paths don't match).

Before, we were checking these two properties in `build_ast`, but we can do so in the `client_edges` transform to avoid generating the model resolver IR when unnecessary.

## This diff

Refactors the existing implementation to check for weak and legacy types in `client_edges` transform.

Reviewed By: captbaritone

Differential Revision: D54556696

fbshipit-source-id: 3e4444856bc8ac66e24bfc12ddcd19745d4c019c

compiler/crates/relay-codegen/src/build_ast.rs

@@ -1281,19 +1281,15 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
     ) -> Vec<ObjectEntry> {
         model_resolvers
             .iter()
-            .filter_map(|model_resolver| {
-                if model_resolver.has_model_instance_field {
-                    let type_name = model_resolver.type_name.item.0;
-                    Some(ObjectEntry {
-                        key: type_name,
-                        value: self.build_client_edge_model_resolver(
-                            model_resolver.type_name,
-                            model_resolver.is_live,
-                            relay_resolver_metadata,
-                        ),
-                    })
-                } else {
-                    None
+            .map(|model_resolver| {
+                let type_name = model_resolver.type_name.item.0;
+                ObjectEntry {
+                    key: type_name,
+                    value: self.build_client_edge_model_resolver(
+                        model_resolver.type_name,
+                        model_resolver.is_live,
+                        relay_resolver_metadata,
+                    ),
                 }
             })
             .collect()
@@ -1803,19 +1799,19 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
                         } else {
                             None
                         }
-                    });
-                    let client_edge_model_resolvers = if let Some(model_resolver_field) = model_resolver_field {
-                        Primitive::Key(model_resolver_field)
-                    } else {
-                        Primitive::Null
-                    };
-                    Primitive::Key(self.object(object! {
-                        kind: Primitive::String(CODEGEN_CONSTANTS.client_edge_to_client_object),
-                        concrete_type: concrete_type,
-                        client_edge_model_resolvers: client_edge_model_resolvers,
-                        client_edge_backing_field_key: backing_field,
-                        client_edge_selections_key: selections_item,
-                    }))
+                        });
+                        let client_edge_model_resolvers = if let Some(model_resolver_field) = model_resolver_field {
+                            Primitive::Key(model_resolver_field)
+                        } else {
+                            Primitive::Null
+                        };
+                        Primitive::Key(self.object(object! {
+                            kind: Primitive::String(CODEGEN_CONSTANTS.client_edge_to_client_object),
+                            concrete_type: concrete_type,
+                            client_edge_model_resolvers: client_edge_model_resolvers,
+                            client_edge_backing_field_key: backing_field,
+                            client_edge_selections_key: selections_item,
+                        }))
                 }
             }
         };

compiler/crates/relay-transforms/src/client_edges.rs

@@ -19,6 +19,7 @@ use docblock_shared::HAS_OUTPUT_TYPE_ARGUMENT_NAME;
 use docblock_shared::LIVE_ARGUMENT_NAME;
 use docblock_shared::RELAY_RESOLVER_DIRECTIVE_NAME;
 use docblock_shared::RELAY_RESOLVER_MODEL_INSTANCE_FIELD;
+use docblock_shared::RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE;
 use graphql_ir::associated_data_impl;
 use graphql_ir::Argument;
 use graphql_ir::ConstantValue;
@@ -90,7 +91,6 @@ associated_data_impl!(ClientEdgeMetadataDirective);
 #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct ClientEdgeModelResolver {
     pub type_name: WithLocation<ObjectName>,
-    pub has_model_instance_field: bool,
     pub is_live: bool,
 }
 
@@ -383,7 +383,22 @@ impl<'program, 'pc> ClientEdgesTransform<'program, 'pc> {
                 }
                 let mut model_resolvers: Vec<ClientEdgeModelResolver> = implementing_objects
                     .iter()
-                    .map(|object_id| self.get_client_edge_model_resolver_for_object(*object_id))
+                    .filter_map(|object_id| {
+                        let model_resolver = self.get_client_edge_model_resolver_for_object(*object_id);
+                        model_resolver.or_else(|| {
+                            if !self.is_weak_type(*object_id) && self.is_resolver_type(*object_id) {
+                                let model_name = self.program.schema.object(*object_id).name;
+                                self.errors.push(Diagnostic::error(
+                                    ValidationMessage::ClientEdgeToClientInterfaceImplementingObjectMissingModelResolver {
+                                        interface_name: interface.name.item,
+                                        type_name: model_name.item,
+                                    },
+                                    model_name.location,
+                                ));
+                            }
+                            None
+                        })
+                    })
                     .collect();
                 model_resolvers.sort();
                 Some(ClientEdgeMetadataDirective::ClientObject {
@@ -402,10 +417,12 @@ impl<'program, 'pc> ClientEdgesTransform<'program, 'pc> {
             }
             Type::Object(object_id) => {
                 let type_name = self.program.schema.object(object_id).name.item;
-                let model_resolver = self.get_client_edge_model_resolver_for_object(object_id);
+                let model_resolvers = self
+                    .get_client_edge_model_resolver_for_object(object_id)
+                    .map_or(vec![], |model_resolver| vec![model_resolver]);
                 Some(ClientEdgeMetadataDirective::ClientObject {
                     type_name: Some(type_name),
-                    model_resolvers: vec![model_resolver],
+                    model_resolvers,
                     unique_id: self.get_key(),
                 })
             }
@@ -415,36 +432,47 @@ impl<'program, 'pc> ClientEdgesTransform<'program, 'pc> {
         }
     }
 
+    fn is_weak_type(&self, object_id: ObjectID) -> bool {
+        let object = self.program.schema.object(object_id);
+        object
+            .directives
+            .named(*RELAY_RESOLVER_WEAK_OBJECT_DIRECTIVE)
+            .is_some()
+    }
+
+    fn is_resolver_type(&self, object_id: ObjectID) -> bool {
+        let object = self.program.schema.object(object_id);
+        object
+            .directives
+            .named(*RELAY_RESOLVER_DIRECTIVE_NAME)
+            .is_some()
+    }
+
     fn get_client_edge_model_resolver_for_object(
         &mut self,
         object_id: ObjectID,
-    ) -> ClientEdgeModelResolver {
+    ) -> Option<ClientEdgeModelResolver> {
         let type_name = self.program.schema.object(object_id).name;
-        let parent_type = self.program.schema.get_type(type_name.item.0).unwrap();
+        let model_type = self.program.schema.get_type(type_name.item.0).unwrap();
         let model_field_id = self
             .program
             .schema
-            .named_field(parent_type, *RELAY_RESOLVER_MODEL_INSTANCE_FIELD);
-        // Note: is_live is only true if the __relay_model_instance field exists on the model field
-        let is_live = if let Some(id) = model_field_id {
-            let model_field = self.program.schema.field(id);
-            let resolver_directive = model_field.directives.named(*RELAY_RESOLVER_DIRECTIVE_NAME);
-            if let Some(resolver_directive) = resolver_directive {
-                resolver_directive
-                    .arguments
-                    .iter()
-                    .any(|arg| arg.name.0 == LIVE_ARGUMENT_NAME.0)
-            } else {
-                false
-            }
-        } else {
-            false
-        };
-        ClientEdgeModelResolver {
-            type_name,
-            has_model_instance_field: model_field_id.is_some(),
-            is_live,
+            .named_field(model_type, *RELAY_RESOLVER_MODEL_INSTANCE_FIELD);
+        // Legacy models (using verbose resolver syntax) do not have a __relay_model_instance field
+        if self.is_weak_type(object_id) {
+            return None;
         }
+        let id = model_field_id?;
+        // Note: is_live is only true if the __relay_model_instance field exists on the model field
+        let model_field = self.program.schema.field(id);
+        let resolver_directive = model_field.directives.named(*RELAY_RESOLVER_DIRECTIVE_NAME);
+        let is_live = resolver_directive.map_or(false, |resolver_directive| {
+            resolver_directive
+                .arguments
+                .iter()
+                .any(|arg| arg.name.0 == LIVE_ARGUMENT_NAME.0)
+        });
+        Some(ClientEdgeModelResolver { type_name, is_live })
     }
 
     fn get_edge_to_server_object_metadata_directive(

compiler/crates/relay-transforms/src/errors.rs

@@ -129,6 +129,14 @@ pub enum ValidationMessage {
     )]
     ClientEdgeToClientInterface,
 
+    #[error(
+        "The client edge pointing to the interface `{interface_name}` with implementing object, `{type_name}`, is missing its corresponding model resolver. The concrete type `{type_name}` and its resolver fields should be defined with the newer dot notation resolver syntax. See https://relay.dev/docs/guides/relay-resolvers/."
+    )]
+    ClientEdgeToClientInterfaceImplementingObjectMissingModelResolver {
+        interface_name: InterfaceName,
+        type_name: ObjectName,
+    },
+
     #[error(
         "Client Edges that reference client-defined union types are not currently supported in Relay."
     )]

compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.expected

@@ -20,6 +20,7 @@ interface ClientOnlyInterface implements Node {
 # Add a concrete type so that we don't trigger an unrelated compiler error.
 type BestFriend implements ClientOnlyInterface {
   id: ID!
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend")
 }
 
 extend type User {
@@ -43,15 +44,14 @@ fragment Foo_user on User {
   #                     "BestFriend",
   #                 ),
   #             },
-  #             has_model_instance_field: false,
   #             is_live: false,
   #         },
   #     ],
   # }
    {
     ...BestFriendResolverFragment_name @__RelayResolverMetadata
     # RelayResolverMetadata {
-    #     field_id: FieldID(525),
+    #     field_id: FieldID(526),
     #     import_path: "BestFriendResolver",
     #     import_name: None,
     #     field_alias: None,

compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.graphql

@@ -19,6 +19,7 @@ interface ClientOnlyInterface implements Node {
 # Add a concrete type so that we don't trigger an unrelated compiler error.
 type BestFriend implements ClientOnlyInterface {
   id: ID!
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend")
 }
 
 extend type User {

compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.expected

@@ -19,6 +19,7 @@ interface ClientOnlyInterface implements Node {
 # Add a concrete type so that we don't trigger an unrelated compiler error.
 type BestFriend implements ClientOnlyInterface {
   id: ID!
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend")
 }
 
 extend type User {

compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-interface.invalid.graphql

@@ -18,6 +18,7 @@ interface ClientOnlyInterface implements Node {
 # Add a concrete type so that we don't trigger an unrelated compiler error.
 type BestFriend implements ClientOnlyInterface {
   id: ID!
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "BestFriendResolver" fragment_name: "BestFriend__id", generated_fragment: true, inject_fragment_data: "id", import_name: "BestFriend")
 }
 
 extend type User {

compiler/crates/relay-transforms/tests/client_edges/fixtures/client-edge-to-client-object.expected

@@ -32,18 +32,7 @@ fragment Foo_user on User {
   #         ),
   #     ),
   #     unique_id: 0,
-  #     model_resolvers: [
-  #         ClientEdgeModelResolver {
-  #             type_name: WithLocation {
-  #                 location: <generated>:7:21,
-  #                 item: ObjectName(
-  #                     "ClientOnlyType",
-  #                 ),
-  #             },
-  #             has_model_instance_field: false,
-  #             is_live: false,
-  #         },
-  #     ],
+  #     model_resolvers: [],
   # }
    {
     ...BestFriendResolverFragment_name @__RelayResolverMetadata

compiler/crates/relay-transforms/tests/client_edges/fixtures/nested-path-with-alias.expected

@@ -38,18 +38,7 @@ fragment Foo_user on ClientUser {
   #         ),
   #     ),
   #     unique_id: 1,
-  #     model_resolvers: [
-  #         ClientEdgeModelResolver {
-  #             type_name: WithLocation {
-  #                 location: <generated>:7:17,
-  #                 item: ObjectName(
-  #                     "ClientUser",
-  #                 ),
-  #             },
-  #             has_model_instance_field: false,
-  #             is_live: false,
-  #         },
-  #     ],
+  #     model_resolvers: [],
   # }
    {
     ...BestFriendFragment @__RelayResolverMetadata
@@ -76,18 +65,7 @@ fragment Foo_user on ClientUser {
       #         ),
       #     ),
       #     unique_id: 0,
-      #     model_resolvers: [
-      #         ClientEdgeModelResolver {
-      #             type_name: WithLocation {
-      #                 location: <generated>:7:17,
-      #                 item: ObjectName(
-      #                     "ClientUser",
-      #                 ),
-      #             },
-      #             has_model_instance_field: false,
-      #             is_live: false,
-      #         },
-      #     ],
+      #     model_resolvers: [],
       # }
        {
         ...BestFriendFragment @__RelayResolverMetadata

compiler/crates/relay-transforms/tests/client_edges/fixtures/nested-path.expected

@@ -38,18 +38,7 @@ fragment Foo_user on ClientUser {
   #         ),
   #     ),
   #     unique_id: 1,
-  #     model_resolvers: [
-  #         ClientEdgeModelResolver {
-  #             type_name: WithLocation {
-  #                 location: <generated>:7:17,
-  #                 item: ObjectName(
-  #                     "ClientUser",
-  #                 ),
-  #             },
-  #             has_model_instance_field: false,
-  #             is_live: false,
-  #         },
-  #     ],
+  #     model_resolvers: [],
   # }
    {
     ...BestFriendFragment @__RelayResolverMetadata
@@ -74,18 +63,7 @@ fragment Foo_user on ClientUser {
       #         ),
       #     ),
       #     unique_id: 0,
-      #     model_resolvers: [
-      #         ClientEdgeModelResolver {
-      #             type_name: WithLocation {
-      #                 location: <generated>:7:17,
-      #                 item: ObjectName(
-      #                     "ClientUser",
-      #                 ),
-      #             },
-      #             has_model_instance_field: false,
-      #             is_live: false,
-      #         },
-      #     ],
+      #     model_resolvers: [],
       # }
        {
         ...BestFriendFragment @__RelayResolverMetadata

compiler/crates/relay-transforms/tests/client_edges/fixtures/output-type.expected

@@ -32,18 +32,7 @@ fragment Foo_user on User {
   #         ),
   #     ),
   #     unique_id: 0,
-  #     model_resolvers: [
-  #         ClientEdgeModelResolver {
-  #             type_name: WithLocation {
-  #                 location: <generated>:7:17,
-  #                 item: ObjectName(
-  #                     "ClientUser",
-  #                 ),
-  #             },
-  #             has_model_instance_field: false,
-  #             is_live: false,
-  #         },
-  #     ],
+  #     model_resolvers: [],
   # }
    {
     ...BestFriendResolverFragment_name @__RelayResolverMetadata

compiler/crates/relay-typegen/tests/generate_flow/fixtures/relay-resolver-with-output-type-client-interface.expected

@@ -25,6 +25,7 @@ interface ClientInterface {
 
 type ClientType implements ClientInterface {
   name: String
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "ClientTypeResolver" fragment_name: "ClientType__id", generated_fragment: true, inject_fragment_data: "id", import_name: "ClientType")
 }
 
 type ClientTypeWithNestedInterface {

compiler/crates/relay-typegen/tests/generate_flow/fixtures/relay-resolver-with-output-type-client-interface.graphql

@@ -24,6 +24,7 @@ interface ClientInterface {
 
 type ClientType implements ClientInterface {
   name: String
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "ClientTypeResolver" fragment_name: "ClientType__id", generated_fragment: true, inject_fragment_data: "id", import_name: "ClientType")
 }
 
 type ClientTypeWithNestedInterface {

compiler/crates/relay-typegen/tests/generate_typescript/fixtures/relay-resolver-with-output-type-client-interface.expected

@@ -25,6 +25,7 @@ interface ClientInterface {
 
 type ClientType implements ClientInterface {
   name: String
+  __relay_model_instance: RelayResolverValue @relay_resolver(import_path: "ClientTypeResolver" fragment_name: "ClientType__id", generated_fragment: true, inject_fragment_data: "id", import_name: "ClientType")
 }
 
 type ClientTypeWithNestedInterface {