Skip to content

Commit

Permalink
Fix selections on abstract type in raw response type
Browse files Browse the repository at this point in the history
  • Loading branch information
tobias-tengler committed Sep 6, 2024
1 parent 2edb82e commit 32f5717
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 22 deletions.
28 changes: 28 additions & 0 deletions compiler/crates/relay-typegen/src/type_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -75,6 +86,17 @@ impl TypeSelection {
}
}

pub(crate) fn get_enclosing_abstract_type(&self) -> Option<Type> {
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,
Expand Down Expand Up @@ -120,6 +142,7 @@ pub(crate) struct RawResponseFragmentSpread {
pub(crate) value: StringKey,
pub(crate) conditional: bool,
pub(crate) concrete_type: Option<Type>,
pub(crate) abstract_type: Option<Type>,
}

#[derive(Debug, Clone)]
Expand All @@ -128,6 +151,7 @@ pub(crate) struct ModuleDirective {
pub(crate) document_name: StringKey,
pub(crate) conditional: bool,
pub(crate) concrete_type: Option<Type>,
pub(crate) abstract_type: Option<Type>,
}

#[derive(Debug, Clone)]
Expand All @@ -137,6 +161,7 @@ pub(crate) struct TypeSelectionLinkedField {
pub(crate) node_selections: TypeSelectionMap,
pub(crate) conditional: bool,
pub(crate) concrete_type: Option<Type>,
pub(crate) abstract_type: Option<Type>,
pub(crate) is_result_type: bool,
}

Expand All @@ -147,6 +172,7 @@ pub(crate) struct TypeSelectionScalarField {
pub(crate) value: AST,
pub(crate) conditional: bool,
pub(crate) concrete_type: Option<Type>,
pub(crate) abstract_type: Option<Type>,
pub(crate) is_result_type: bool,
}

Expand All @@ -155,13 +181,15 @@ pub(crate) struct TypeSelectionInlineFragment {
pub(crate) fragment_name: FragmentDefinitionName,
pub(crate) conditional: bool,
pub(crate) concrete_type: Option<Type>,
pub(crate) abstract_type: Option<Type>,
}

#[derive(Debug, Clone)]
pub(crate) struct TypeSelectionFragmentSpread {
pub(crate) fragment_name: FragmentDefinitionName,
pub(crate) conditional: bool,
pub(crate) concrete_type: Option<Type>,
pub(crate) abstract_type: Option<Type>,
// 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.
Expand Down
94 changes: 88 additions & 6 deletions compiler/crates/relay-typegen/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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;
Expand Down Expand Up @@ -321,6 +322,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
Expand Down Expand Up @@ -794,6 +796,7 @@ fn visit_relay_resolver(
value: resolver_type,
conditional: false,
concrete_type: None,
abstract_type: None,
is_result_type: false,
}));
}
Expand Down Expand Up @@ -891,6 +894,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 {
Expand All @@ -899,12 +903,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
Expand Down Expand Up @@ -986,6 +992,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,
})]
Expand Down Expand Up @@ -1082,6 +1089,7 @@ fn visit_actor_change(
)))),
conditional: false,
concrete_type: None,
abstract_type: None,
is_result_type: false,
}));
}
Expand Down Expand Up @@ -1143,11 +1151,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);
}
Expand Down Expand Up @@ -1189,6 +1203,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),
}));
}
Expand Down Expand Up @@ -1247,6 +1262,7 @@ fn visit_scalar_field(
)),
conditional: false,
concrete_type: None,
abstract_type: None,
is_result_type: is_result_type_directive(&scalar_field.directives),
}));
}
Expand All @@ -1262,6 +1278,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),
}));
}
Expand Down Expand Up @@ -1620,21 +1637,28 @@ pub(crate) fn raw_response_selections_to_babel(
) -> AST {
let mut base_fields = Vec::new();
let mut by_concrete_type: IndexMap<Type, Vec<TypeSelection>> = Default::default();
let mut by_abstract_type: IndexMap<Type, Vec<TypeSelection>> = Default::default();

for selection in selections {
if let Some(concrete_type) = selection.get_enclosing_concrete_type() {
by_concrete_type
.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()));
}

Expand All @@ -1649,6 +1673,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
Expand Down Expand Up @@ -1678,9 +1725,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| {
Expand Down Expand Up @@ -2238,6 +2317,7 @@ pub(crate) fn raw_response_visit_selections(
value: spread_type,
conditional: false,
concrete_type: None,
abstract_type: None,
},
))
}
Expand Down Expand Up @@ -2595,6 +2675,7 @@ fn group_refs(props: impl Iterator<Item = TypeSelection>) -> impl Iterator<Item
special_field: None,
conditional: false,
concrete_type: None,
abstract_type: None,
is_result_type: false,
}));
}
Expand All @@ -2606,6 +2687,7 @@ fn group_refs(props: impl Iterator<Item = TypeSelection>) -> impl Iterator<Item
special_field: None,
conditional: false,
concrete_type: None,
abstract_type: None,
is_result_type: false,
}));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,9 @@ export type ExampleQuery$data = {|
export type ExampleQuery$rawResponse = {|
+maybeNode: ?({|
+__typename: "FakeNode",
+__isNode: "FakeNode",
+id: string,
|} | {|
+__typename: "NonNode",
+__isNode: "NonNode",
+id: string,
+name: ?string,
|} | {|
+__typename: "Story",
Expand All @@ -48,8 +45,8 @@ export type ExampleQuery$rawResponse = {|
+id: string,
|} | {|
+__typename: string,
+__isNode: string,
+id: string,
+__isNode?: string,
+id?: string,
|}),
|};
export type ExampleQuery = {|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,9 @@ export type ExampleQuery$data = {
export type ExampleQuery$rawResponse = {
readonly maybeNode: {
readonly __typename: "FakeNode";
readonly __isNode: "FakeNode";
readonly id: string;
} | {
readonly __typename: "NonNode";
readonly __isNode: "NonNode";
readonly id: string;
readonly name: string | null | undefined;
} | {
readonly __typename: "Story";
Expand All @@ -48,8 +45,8 @@ export type ExampleQuery$rawResponse = {
readonly id: string;
} | {
readonly __typename: string;
readonly __isNode: string;
readonly id: string;
readonly __isNode?: string;
readonly id?: string;
} | null | undefined;
};
export type ExampleQuery = {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 32f5717

Please sign in to comment.