Skip to content

Commit d9967e8

Browse files
tobias-tenglerfacebook-github-bot
authored andcommitted
Do not emit union type for __typename selection on non-abstract type (#4923)
Summary: I've noticed that selecting only the `__typename` field on an object type ```graphql fragment PageFragment on Page { __typename } ``` currently leads to the generation of a union type: ```typescript export type PageFragment$data = { readonly __typename: "Page"; readonly " $fragmentType": "PageFragment"; } | { // This will never be '%other', but we need some // value in case none of the concrete values match. readonly __typename: "%other"; readonly " $fragmentType": "PageFragment"; }; ``` This PR corrects this behavior and no longer emits a union in this case: ```typescript export type PageFragment$data = { readonly __typename: "Page"; readonly " $fragmentType": "PageFragment"; }; ``` Pull Request resolved: #4923 Reviewed By: captbaritone Differential Revision: D70718298 Pulled By: monicatang fbshipit-source-id: 59a8b9a128b34ac6a8ebdf2f365b6d221b511d77
1 parent cc4cf76 commit d9967e8

File tree

8 files changed

+21
-72
lines changed

8 files changed

+21
-72
lines changed

compiler/crates/relay-typegen/src/visit.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,7 @@ fn visit_actor_change(
10931093
value: AST::Nullable(Box::new(AST::ActorChangePoint(Box::new(
10941094
selections_to_babel(
10951095
typegen_context,
1096+
&field.type_.inner(),
10961097
linked_field_selections.into_iter(),
10971098
MaskStatus::Masked,
10981099
None,
@@ -1368,6 +1369,7 @@ fn visit_condition(
13681369
#[allow(clippy::too_many_arguments)]
13691370
pub(crate) fn get_data_type(
13701371
typegen_context: &'_ TypegenContext<'_>,
1372+
concrete_type: &Type,
13711373
selections: impl Iterator<Item = TypeSelection>,
13721374
mask_status: MaskStatus,
13731375
fragment_type_name: Option<StringKey>,
@@ -1381,6 +1383,7 @@ pub(crate) fn get_data_type(
13811383
) -> AST {
13821384
let mut data_type = selections_to_babel(
13831385
typegen_context,
1386+
concrete_type,
13841387
selections,
13851388
mask_status,
13861389
fragment_type_name,
@@ -1402,6 +1405,7 @@ pub(crate) fn get_data_type(
14021405
#[allow(clippy::too_many_arguments)]
14031406
fn selections_to_babel(
14041407
typegen_context: &'_ TypegenContext<'_>,
1408+
concrete_type: &Type,
14051409
selections: impl Iterator<Item = TypeSelection>,
14061410
mask_status: MaskStatus,
14071411
fragment_type_name: Option<StringKey>,
@@ -1441,7 +1445,7 @@ fn selections_to_babel(
14411445
}
14421446
}
14431447

1444-
if should_emit_discriminated_union(&by_concrete_type, &base_fields) {
1448+
if should_emit_discriminated_union(concrete_type, &by_concrete_type, &base_fields) {
14451449
get_discriminated_union_ast(
14461450
by_concrete_type,
14471451
&base_fields,
@@ -1660,11 +1664,15 @@ fn get_discriminated_union_ast(
16601664
///
16611665
/// If this condition passes, we emit a discriminated union
16621666
fn should_emit_discriminated_union(
1667+
concrete_type: &Type,
16631668
by_concrete_type: &IndexMap<Type, Vec<TypeSelection>>,
16641669
base_fields: &IndexMap<StringKey, TypeSelection>,
16651670
) -> bool {
1666-
!by_concrete_type.is_empty()
1667-
&& base_fields.values().all(TypeSelection::is_typename)
1671+
if by_concrete_type.is_empty() || !concrete_type.is_abstract_type() {
1672+
return false;
1673+
}
1674+
1675+
base_fields.values().all(TypeSelection::is_typename)
16681676
&& (base_fields.values().any(TypeSelection::is_typename)
16691677
|| by_concrete_type
16701678
.values()
@@ -1885,6 +1893,7 @@ fn make_prop(
18851893

18861894
let getter_object_props = selections_to_babel(
18871895
typegen_context,
1896+
&linked_field.node_type.inner(),
18881897
no_fragments.into_iter(),
18891898
mask_status,
18901899
None,
@@ -1976,6 +1985,7 @@ fn make_prop(
19761985
} else {
19771986
let object_props = selections_to_babel(
19781987
typegen_context,
1988+
&linked_field.node_type.inner(),
19791989
hashmap_into_values(linked_field.node_selections),
19801990
mask_status,
19811991
None,

compiler/crates/relay-typegen/src/write.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ pub(crate) fn write_operation_type_exports_section(
139139

140140
let mut data_type = get_data_type(
141141
typegen_context,
142+
&typegen_operation.type_,
142143
type_selections.into_iter(),
143144
MaskStatus::Masked, // Queries are never unmasked
144145
None,
@@ -461,6 +462,7 @@ pub(crate) fn write_fragment_type_exports_section(
461462

462463
let mut data_type = get_data_type(
463464
typegen_context,
465+
&fragment_definition.type_condition,
464466
type_selections.into_iter(),
465467
mask_status,
466468
if mask_status == MaskStatus::Unmasked {
@@ -607,12 +609,9 @@ fn write_fragment_imports(
607609
),
608610
};
609611

610-
let should_write_current_referenced_fragment = fragment_name_to_skip
611-
.map_or(true, |fragment_name_to_skip| {
612-
fragment_name_to_skip != current_referenced_fragment
613-
});
614-
615-
if !should_write_current_referenced_fragment {
612+
let should_skip_writing_current_referenced_fragment =
613+
fragment_name_to_skip == Some(current_referenced_fragment);
614+
if should_skip_writing_current_referenced_fragment {
616615
continue;
617616
}
618617

@@ -1111,7 +1110,7 @@ fn write_concrete_validator_function(
11111110
}
11121111

11131112
fn is_plural(node: &FragmentDefinition) -> bool {
1114-
RelayDirective::find(&node.directives).map_or(false, |relay_directive| relay_directive.plural)
1113+
RelayDirective::find(&node.directives).is_some_and(|relay_directive| relay_directive.plural)
11151114
}
11161115

11171116
fn has_fragment_spread(selections: &[Selection]) -> bool {

compiler/crates/relay-typegen/tests/generate_flow/fixtures/fragment-spread.expected

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,6 @@ declare export opaque type PageFragment$fragmentType: FragmentType;
113113
export type PageFragment$data = {|
114114
+__typename: "Page",
115115
+$fragmentType: PageFragment$fragmentType,
116-
|} | {|
117-
// This will never be '%other', but we need some
118-
// value in case none of the concrete values match.
119-
+__typename: "%other",
120-
+$fragmentType: PageFragment$fragmentType,
121116
|};
122117
export type PageFragment$key = {
123118
+$data?: PageFragment$data,
@@ -130,11 +125,6 @@ declare export opaque type PictureFragment$fragmentType: FragmentType;
130125
export type PictureFragment$data = {|
131126
+__typename: "Image",
132127
+$fragmentType: PictureFragment$fragmentType,
133-
|} | {|
134-
// This will never be '%other', but we need some
135-
// value in case none of the concrete values match.
136-
+__typename: "%other",
137-
+$fragmentType: PictureFragment$fragmentType,
138128
|};
139129
export type PictureFragment$key = {
140130
+$data?: PictureFragment$data,
@@ -147,11 +137,6 @@ declare export opaque type UserFrag1$fragmentType: FragmentType;
147137
export type UserFrag1$data = {|
148138
+__typename: "User",
149139
+$fragmentType: UserFrag1$fragmentType,
150-
|} | {|
151-
// This will never be '%other', but we need some
152-
// value in case none of the concrete values match.
153-
+__typename: "%other",
154-
+$fragmentType: UserFrag1$fragmentType,
155140
|};
156141
export type UserFrag1$key = {
157142
+$data?: UserFrag1$data,
@@ -164,11 +149,6 @@ declare export opaque type UserFrag2$fragmentType: FragmentType;
164149
export type UserFrag2$data = {|
165150
+__typename: "User",
166151
+$fragmentType: UserFrag2$fragmentType,
167-
|} | {|
168-
// This will never be '%other', but we need some
169-
// value in case none of the concrete values match.
170-
+__typename: "%other",
171-
+$fragmentType: UserFrag2$fragmentType,
172152
|};
173153
export type UserFrag2$key = {
174154
+$data?: UserFrag2$data,

compiler/crates/relay-typegen/tests/generate_flow/fixtures/inline-fragment.expected

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,6 @@ declare export opaque type SomeFragment$fragmentType: FragmentType;
139139
export type SomeFragment$data = {|
140140
+__typename: "User",
141141
+$fragmentType: SomeFragment$fragmentType,
142-
|} | {|
143-
// This will never be '%other', but we need some
144-
// value in case none of the concrete values match.
145-
+__typename: "%other",
146-
+$fragmentType: SomeFragment$fragmentType,
147142
|};
148143
export type SomeFragment$key = {
149144
+$data?: SomeFragment$data,

compiler/crates/relay-typegen/tests/generate_typescript/fixtures/fragment-spread.expected

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,6 @@ import { FragmentRefs } from "relay-runtime";
101101
export type PageFragment$data = {
102102
readonly __typename: "Page";
103103
readonly " $fragmentType": "PageFragment";
104-
} | {
105-
// This will never be '%other', but we need some
106-
// value in case none of the concrete values match.
107-
readonly __typename: "%other";
108-
readonly " $fragmentType": "PageFragment";
109104
};
110105
export type PageFragment$key = {
111106
readonly " $data"?: PageFragment$data;
@@ -116,11 +111,6 @@ import { FragmentRefs } from "relay-runtime";
116111
export type PictureFragment$data = {
117112
readonly __typename: "Image";
118113
readonly " $fragmentType": "PictureFragment";
119-
} | {
120-
// This will never be '%other', but we need some
121-
// value in case none of the concrete values match.
122-
readonly __typename: "%other";
123-
readonly " $fragmentType": "PictureFragment";
124114
};
125115
export type PictureFragment$key = {
126116
readonly " $data"?: PictureFragment$data;
@@ -131,11 +121,6 @@ import { FragmentRefs } from "relay-runtime";
131121
export type UserFrag1$data = {
132122
readonly __typename: "User";
133123
readonly " $fragmentType": "UserFrag1";
134-
} | {
135-
// This will never be '%other', but we need some
136-
// value in case none of the concrete values match.
137-
readonly __typename: "%other";
138-
readonly " $fragmentType": "UserFrag1";
139124
};
140125
export type UserFrag1$key = {
141126
readonly " $data"?: UserFrag1$data;
@@ -146,11 +131,6 @@ import { FragmentRefs } from "relay-runtime";
146131
export type UserFrag2$data = {
147132
readonly __typename: "User";
148133
readonly " $fragmentType": "UserFrag2";
149-
} | {
150-
// This will never be '%other', but we need some
151-
// value in case none of the concrete values match.
152-
readonly __typename: "%other";
153-
readonly " $fragmentType": "UserFrag2";
154134
};
155135
export type UserFrag2$key = {
156136
readonly " $data"?: UserFrag2$data;

compiler/crates/relay-typegen/tests/generate_typescript/fixtures/inline-fragment.expected

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,6 @@ import { FragmentRefs } from "relay-runtime";
129129
export type SomeFragment$data = {
130130
readonly __typename: "User";
131131
readonly " $fragmentType": "SomeFragment";
132-
} | {
133-
// This will never be '%other', but we need some
134-
// value in case none of the concrete values match.
135-
readonly __typename: "%other";
136-
readonly " $fragmentType": "SomeFragment";
137132
};
138133
export type SomeFragment$key = {
139134
readonly " $data"?: SomeFragment$data;

packages/relay-runtime/store/__tests__/resolvers/__generated__/UserAlwaysThrowsResolver.graphql.js

Lines changed: 1 addition & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/relay-runtime/store/__tests__/resolvers/__generated__/UserAnotherClientEdgeResolver.graphql.js

Lines changed: 1 addition & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)