From b8833820b2b141064cc80e49c3a901016ca4173d Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Tue, 20 Aug 2024 20:50:19 +0200 Subject: [PATCH] Fix selections on abstract type in raw response type --- .../relay-typegen/src/type_selection.rs | 28 ++++++ compiler/crates/relay-typegen/src/visit.rs | 94 +++++++++++++++++-- .../query-with-raw-response-union.expected | 7 +- .../query-with-raw-response-union.expected | 7 +- ...tIsEntityInlineFragmentMutation.graphql.js | 6 +- ...tIsEntitySpreadFragmentMutation.graphql.js | 6 +- 6 files changed, 126 insertions(+), 22 deletions(-) diff --git a/compiler/crates/relay-typegen/src/type_selection.rs b/compiler/crates/relay-typegen/src/type_selection.rs index 59899635f77c3..732bc6ca6f6ca 100644 --- a/compiler/crates/relay-typegen/src/type_selection.rs +++ b/compiler/crates/relay-typegen/src/type_selection.rs @@ -42,6 +42,17 @@ impl TypeSelection { } } + pub(crate) fn set_abstract_type(&mut self, type_: Type) { + match self { + TypeSelection::LinkedField(l) => l.abstract_type = Some(type_), + TypeSelection::ScalarField(s) => s.abstract_type = Some(type_), + TypeSelection::InlineFragment(f) => f.abstract_type = Some(type_), + TypeSelection::FragmentSpread(f) => f.abstract_type = Some(type_), + TypeSelection::ModuleDirective(m) => m.abstract_type = Some(type_), + TypeSelection::RawResponseFragmentSpread(f) => f.abstract_type = Some(type_), + } + } + pub(crate) fn set_conditional(&mut self, conditional: bool) { match self { TypeSelection::LinkedField(l) => l.conditional = conditional, @@ -75,6 +86,17 @@ impl TypeSelection { } } + pub(crate) fn get_enclosing_abstract_type(&self) -> Option { + match self { + TypeSelection::LinkedField(l) => l.abstract_type, + TypeSelection::ScalarField(s) => s.abstract_type, + TypeSelection::FragmentSpread(f) => f.abstract_type, + TypeSelection::InlineFragment(f) => f.abstract_type, + TypeSelection::ModuleDirective(m) => m.abstract_type, + TypeSelection::RawResponseFragmentSpread(f) => f.abstract_type, + } + } + pub(crate) fn is_typename(&self) -> bool { matches!( self, @@ -120,6 +142,7 @@ pub(crate) struct RawResponseFragmentSpread { pub(crate) value: StringKey, pub(crate) conditional: bool, pub(crate) concrete_type: Option, + pub(crate) abstract_type: Option, } #[derive(Debug, Clone)] @@ -128,6 +151,7 @@ pub(crate) struct ModuleDirective { pub(crate) document_name: StringKey, pub(crate) conditional: bool, pub(crate) concrete_type: Option, + pub(crate) abstract_type: Option, } #[derive(Debug, Clone)] @@ -137,6 +161,7 @@ pub(crate) struct TypeSelectionLinkedField { pub(crate) node_selections: TypeSelectionMap, pub(crate) conditional: bool, pub(crate) concrete_type: Option, + pub(crate) abstract_type: Option, pub(crate) is_result_type: bool, } @@ -147,6 +172,7 @@ pub(crate) struct TypeSelectionScalarField { pub(crate) value: AST, pub(crate) conditional: bool, pub(crate) concrete_type: Option, + pub(crate) abstract_type: Option, pub(crate) is_result_type: bool, } @@ -155,6 +181,7 @@ pub(crate) struct TypeSelectionInlineFragment { pub(crate) fragment_name: FragmentDefinitionName, pub(crate) conditional: bool, pub(crate) concrete_type: Option, + pub(crate) abstract_type: Option, } #[derive(Debug, Clone)] @@ -162,6 +189,7 @@ pub(crate) struct TypeSelectionFragmentSpread { pub(crate) fragment_name: FragmentDefinitionName, pub(crate) conditional: bool, pub(crate) concrete_type: Option, + pub(crate) abstract_type: Option, // Why are we using TypeSelectionInfo instead of re-using concrete_type? // Because concrete_type is poorly named and does not refer to the concrete // type of the fragment spread. diff --git a/compiler/crates/relay-typegen/src/visit.rs b/compiler/crates/relay-typegen/src/visit.rs index e7fb2f9162846..38567d7227ec8 100644 --- a/compiler/crates/relay-typegen/src/visit.rs +++ b/compiler/crates/relay-typegen/src/visit.rs @@ -68,6 +68,7 @@ use schema::ScalarID; use schema::Schema; use schema::Type; use schema::TypeReference; +use schema::TypeWithFields; use crate::type_selection::ModuleDirective; use crate::type_selection::RawResponseFragmentSpread; @@ -319,6 +320,7 @@ fn visit_fragment_spread( fragment_name: name, conditional: false, concrete_type: None, + abstract_type: None, type_condition_info: get_type_condition_info(fragment_spread), is_updatable_fragment_spread: fragment_spread .directives @@ -743,6 +745,7 @@ fn visit_relay_resolver( value: resolver_type, conditional: false, concrete_type: None, + abstract_type: None, is_result_type: false, })); } @@ -840,6 +843,7 @@ fn visit_inline_fragment( value: AST::Nullable(Box::new(AST::String)), conditional: false, concrete_type: None, + abstract_type: None, is_result_type: false, })); type_selections.push(TypeSelection::ScalarField(TypeSelectionScalarField { @@ -848,12 +852,14 @@ fn visit_inline_fragment( value: AST::Nullable(Box::new(AST::String)), conditional: false, concrete_type: None, + abstract_type: None, is_result_type: false, })); type_selections.push(TypeSelection::InlineFragment(TypeSelectionInlineFragment { fragment_name: name, conditional: false, concrete_type: None, + abstract_type: None, })); } else if inline_fragment .directives @@ -935,6 +941,7 @@ fn visit_inline_fragment( node_selections: selections_to_map(inline_selections.into_iter(), true), conditional: false, concrete_type: None, + abstract_type: None, // @catch cannot be used directly on an inline fragment. is_result_type: false, })] @@ -1031,6 +1038,7 @@ fn visit_actor_change( )))), conditional: false, concrete_type: None, + abstract_type: None, is_result_type: false, })); } @@ -1092,11 +1100,17 @@ fn raw_response_visit_inline_fragment( document_name: module_metadata.key, conditional: false, concrete_type: None, + abstract_type: None, })); return; } + if let Some(type_condition) = inline_fragment.type_condition { - if !type_condition.is_abstract_type() { + if type_condition.is_abstract_type() { + for selection in &mut selections { + selection.set_abstract_type(type_condition); + } + } else { for selection in &mut selections { selection.set_concrete_type(type_condition); } @@ -1138,6 +1152,7 @@ fn gen_visit_linked_field( node_selections: selections_to_map(selections.into_iter(), true), conditional: false, concrete_type: None, + abstract_type: None, is_result_type: is_result_type_directive(&linked_field.directives), })); } @@ -1196,6 +1211,7 @@ fn visit_scalar_field( )), conditional: false, concrete_type: None, + abstract_type: None, is_result_type: is_result_type_directive(&scalar_field.directives), })); } @@ -1211,6 +1227,7 @@ fn visit_scalar_field( value: ast, conditional: false, concrete_type: None, + abstract_type: None, is_result_type: is_result_type_directive(&scalar_field.directives), })); } @@ -1569,6 +1586,7 @@ pub(crate) fn raw_response_selections_to_babel( ) -> AST { let mut base_fields = Vec::new(); let mut by_concrete_type: IndexMap> = Default::default(); + let mut by_abstract_type: IndexMap> = Default::default(); for selection in selections { if let Some(concrete_type) = selection.get_enclosing_concrete_type() { @@ -1576,14 +1594,20 @@ pub(crate) fn raw_response_selections_to_babel( .entry(concrete_type) .or_default() .push(selection); + } else if let Some(abstract_type) = selection.get_enclosing_abstract_type() { + by_abstract_type + .entry(abstract_type) + .or_default() + .push(selection); } else { base_fields.push(selection); } } - if base_fields.is_empty() && by_concrete_type.is_empty() { - // base fields and per-type fields are all empty: this can only occur because the only selection was a - // @no_inline fragment. in this case, emit a single, empty ExactObject since nothing was selected + if base_fields.is_empty() && by_abstract_type.is_empty() && by_concrete_type.is_empty() { + // base fields, fields on abstract types and per-type fields are all empty: + // this can only occur because the only selection was a @no_inline fragment. + // in this case, emit a single, empty ExactObject since nothing was selected. return AST::ExactObject(ExactObject::new(Default::default())); } @@ -1598,6 +1622,29 @@ pub(crate) fn raw_response_selections_to_babel( selections_to_map(selections.into_iter(), false), false, ); + + if !by_abstract_type.is_empty() { + if let Some(object) = concrete_type + .get_object_id() + .map(|id| typegen_context.schema.object(id)) + { + for (abstract_type, abstract_selections) in &by_abstract_type { + if let Some(interface_id) = abstract_type.get_interface_id() { + if object.interfaces().contains(&interface_id) { + merge_selection_maps( + &mut base_fields_map, + selections_to_map( + abstract_selections.clone().into_iter(), + false, + ), + false, + ); + } + } + } + } + } + let merged_selections: Vec<_> = hashmap_into_values(base_fields_map).collect(); types.push(AST::ExactObject(ExactObject::new( merged_selections @@ -1627,9 +1674,41 @@ pub(crate) fn raw_response_selections_to_babel( } } - if !base_fields.is_empty() { + if !base_fields.is_empty() || !by_abstract_type.is_empty() { + let mut base_fields_map = selections_to_map(base_fields.clone().into_iter(), false); + + if !by_abstract_type.is_empty() { + for (abstract_type, abstract_selections) in by_abstract_type { + let mut is_conditional = true; + + if let Some(concrete_type) = concrete_type { + if abstract_type == concrete_type { + is_conditional = false; + } else { + if let Some(object) = concrete_type + .get_object_id() + .map(|id| typegen_context.schema.object(id)) + { + if let Some(interface_id) = abstract_type.get_interface_id() { + if object.interfaces().contains(&interface_id) { + is_conditional = false; + } + } + } + } + } + + merge_selection_maps( + &mut base_fields_map, + selections_to_map(abstract_selections.into_iter(), false), + is_conditional, + ); + } + } + + let merged_selections: Vec<_> = hashmap_into_values(base_fields_map).collect(); types.push(AST::ExactObject(ExactObject::new( - base_fields + merged_selections .iter() .cloned() .map(|selection| { @@ -2187,6 +2266,7 @@ pub(crate) fn raw_response_visit_selections( value: spread_type, conditional: false, concrete_type: None, + abstract_type: None, }, )) } @@ -2544,6 +2624,7 @@ fn group_refs(props: impl Iterator) -> impl Iterator) -> impl Iterator> + * @generated SignedSource<<3b0a0abb302eee61f681b49c0fbbb31d>> * @flow * @lightSyntaxTransform * @nogrep @@ -35,9 +35,9 @@ export type validateMutationTestIsEntityInlineFragmentMutation$rawResponse = {| +actorNameChange: ?{| +actor: ?{| +__typename: string, - +__isEntity: string, + +__isEntity?: string, +id: string, - +url: ?string, + +url?: ?string, |}, |}, |}; diff --git a/packages/relay-runtime/mutations/__tests__/__generated__/validateMutationTestIsEntitySpreadFragmentMutation.graphql.js b/packages/relay-runtime/mutations/__tests__/__generated__/validateMutationTestIsEntitySpreadFragmentMutation.graphql.js index 5e6f311862944..8bba5cda3cfd5 100644 --- a/packages/relay-runtime/mutations/__tests__/__generated__/validateMutationTestIsEntitySpreadFragmentMutation.graphql.js +++ b/packages/relay-runtime/mutations/__tests__/__generated__/validateMutationTestIsEntitySpreadFragmentMutation.graphql.js @@ -6,7 +6,7 @@ * * @oncall relay * - * @generated SignedSource<<0248d6513837bacbfbff93448840442e>> + * @generated SignedSource<<9cfd324f1baacdd1c6b44d590aa8dee7>> * @flow * @lightSyntaxTransform * @nogrep @@ -36,9 +36,9 @@ export type validateMutationTestIsEntitySpreadFragmentMutation$rawResponse = {| +actorNameChange: ?{| +actor: ?{| +__typename: string, - +__isEntity: string, + +__isEntity?: string, +id: string, - +url: ?string, + +url?: ?string, |}, |}, |};