Skip to content

Commit

Permalink
Fix type generation of nullable types in TypeScript (#4380)
Browse files Browse the repository at this point in the history
Summary:
Closes: #4379

## Release Notes

Previously, the relay-compiler would generate a union of the actual type and `null` (`T | null`) for nullable fields when targeting TypeScript. This was not correct, since the Relay store can also return `undefined` in the case of missing data. This version now produces a union of the actual type, `null` and `undefined` (`T | null | undefined`).
Consequently you now also have to check both the `null` and `undefined` case, before accessing a field's value:

```ts
const data = useLazyLoadQuery(/* ... */);

// Notice the `!=` instead of `!==` to check against null and undefined
if (data.nullableField != null)
{
  // Safely access data.nullableField
}
```

Since this is a pretty big change, we're offering the `typescriptExcludeUndefinedFromNullableUnion` feature flag in the `relay-compiler` config to keep the old type generation behavior for now:

```json
{
  "language": "typescript",
  "typescriptExcludeUndefinedFromNullableUnion": true
}
```

Just be aware that we will remove this feature flag in a future release and that you might run into an unexpected `undefined`, if you're not checking for `undefined` before accessing a field - especially if it's a client-only field.

Pull Request resolved: #4380

Reviewed By: alunyov

Differential Revision: D48989476

Pulled By: captbaritone

fbshipit-source-id: 52c5f215298de50e8506245a9d6696e1e935fc2a
  • Loading branch information
tobias-tengler authored and facebook-github-bot committed Oct 6, 2023
1 parent 9eb2747 commit cc47bc9
Show file tree
Hide file tree
Showing 55 changed files with 457 additions and 440 deletions.
6 changes: 6 additions & 0 deletions compiler/crates/relay-config/src/typegen_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ pub struct TypegenConfig {
/// This option enables emitting es modules artifacts.
#[serde(default)]
pub eager_es_modules: bool,

/// Keep the previous compiler behavior by outputting an union
/// of the raw type and null, and not the **correct** behavior
/// of an union with the raw type, null and undefined.
#[serde(default)]
pub typescript_exclude_undefined_from_nullable_union: bool,
}

#[derive(Default, Debug, Serialize, Deserialize, Clone, Copy)]
Expand Down
17 changes: 14 additions & 3 deletions compiler/crates/relay-typegen/src/typescript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::KEY_FRAGMENT_TYPE;
pub struct TypeScriptPrinter {
result: String,
use_import_type_syntax: bool,
include_undefined_in_nullable_union: bool,
indentation: usize,
}

Expand Down Expand Up @@ -170,6 +171,8 @@ impl TypeScriptPrinter {
result: String::new(),
indentation: 0,
use_import_type_syntax: config.use_import_type_syntax,
include_undefined_in_nullable_union: !config
.typescript_exclude_undefined_from_nullable_union,
}
}

Expand Down Expand Up @@ -212,13 +215,21 @@ impl TypeScriptPrinter {

fn write_nullable(&mut self, of_type: &AST) -> FmtResult {
let null_type = AST::RawType("null".intern());
let undefined_type = AST::RawType("undefined".intern());
if let AST::Union(members) = of_type {
let mut new_members = Vec::with_capacity(members.len() + 1);
new_members.extend_from_slice(members);
new_members.push(null_type);
if self.include_undefined_in_nullable_union {
new_members.push(undefined_type);
}
self.write_union(&*new_members)?;
} else {
self.write_union(&*vec![of_type.clone(), null_type])?;
let mut union_members = vec![of_type.clone(), null_type];
if self.include_undefined_in_nullable_union {
union_members.push(undefined_type)
}
self.write_union(&*union_members)?;
}
Ok(())
}
Expand Down Expand Up @@ -387,14 +398,14 @@ mod tests {
fn nullable_type() {
assert_eq!(
print_type(&AST::Nullable(Box::new(AST::String))),
"string | null".to_string()
"string | null | undefined".to_string()
);

assert_eq!(
print_type(&AST::Nullable(Box::new(AST::Union(SortedASTList::new(
vec![AST::String, AST::Number],
))))),
"string | number | null"
"string | number | null | undefined"
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ export type MyQuery$variables = {};
export type MyQuery$data = {
readonly me: {
readonly my_inline_fragment: {
readonly name: string | null;
} | null;
readonly name: string | null | undefined;
} | null | undefined;
readonly my_user: {
readonly " $fragmentSpreads": FragmentRefs<"MyUserFragment">;
} | null;
} | null;
} | null | undefined;
} | null | undefined;
};
export type MyQuery$rawResponse = {
readonly me: {
readonly id: string;
readonly name: string | null;
} | null;
readonly name: string | null | undefined;
} | null | undefined;
};
export type MyQuery = {
rawResponse: MyQuery$rawResponse;
Expand All @@ -38,7 +38,7 @@ export type MyQuery = {
-------------------------------------------------------------------------------
import { FragmentRefs } from "relay-runtime";
export type MyUserFragment$data = {
readonly name: string | null;
readonly name: string | null | undefined;
readonly " $fragmentType": "MyUserFragment";
};
export type MyUserFragment$key = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export type RelayReaderNamedFragmentsTest2Query$data = {
readonly node: {
readonly named_fragment: {
readonly " $fragmentSpreads": FragmentRefs<"RelayReaderNamedFragmentsTest_maybe_node_interface">;
} | null;
} | null;
} | null | undefined;
} | null | undefined;
};
export type RelayReaderNamedFragmentsTest2Query = {
response: RelayReaderNamedFragmentsTest2Query$data;
Expand All @@ -25,7 +25,7 @@ export type RelayReaderNamedFragmentsTest2Query = {
-------------------------------------------------------------------------------
import { FragmentRefs } from "relay-runtime";
export type RelayReaderNamedFragmentsTest_maybe_node_interface$data = {
readonly name: string | null;
readonly name: string | null | undefined;
readonly " $fragmentType": "RelayReaderNamedFragmentsTest_maybe_node_interface";
};
export type RelayReaderNamedFragmentsTest_maybe_node_interface$key = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export type RelayReaderNamedFragmentsTest2Query$data = {
readonly me: {
readonly named_fragment: {
readonly " $fragmentSpreads": FragmentRefs<"RelayReaderNamedFragmentsTest_user">;
} | null;
} | null;
} | null | undefined;
} | null | undefined;
};
export type RelayReaderNamedFragmentsTest2Query = {
response: RelayReaderNamedFragmentsTest2Query$data;
Expand All @@ -25,7 +25,7 @@ export type RelayReaderNamedFragmentsTest2Query = {
-------------------------------------------------------------------------------
import { FragmentRefs } from "relay-runtime";
export type RelayReaderNamedFragmentsTest_user$data = {
readonly name: string | null;
readonly name: string | null | undefined;
readonly " $fragmentType": "RelayReaderNamedFragmentsTest_user";
};
export type RelayReaderNamedFragmentsTest_user$key = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ fragment Foo on User {
import { FragmentRefs } from "relay-runtime";
export type Foo$data = {
readonly named_fragment: {
readonly name: string | null;
} | null;
readonly name: string | null | undefined;
} | null | undefined;
readonly " $fragmentType": "Foo";
};
export type Foo$key = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ export type RelayReaderNamedFragmentsTest2Query$data = {
readonly me: {
readonly id: string;
readonly named_fragment: {
readonly name: string | null;
} | null;
} | null;
readonly name: string | null | undefined;
} | null | undefined;
} | null | undefined;
};
export type RelayReaderNamedFragmentsTest2Query = {
response: RelayReaderNamedFragmentsTest2Query$data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ export type RelayReaderNamedFragmentsTest2Query$data = {
readonly named_fragment: {
readonly me: {
readonly id: string;
readonly name: string | null;
} | null;
} | null;
readonly name: string | null | undefined;
} | null | undefined;
} | null | undefined;
};
export type RelayReaderNamedFragmentsTest2Query = {
response: RelayReaderNamedFragmentsTest2Query$data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ export type RelayReaderNamedFragmentsTest2Query$data = {
readonly me: {
readonly id: string;
readonly named_fragment: {
readonly name: string | null;
} | null;
} | null;
readonly name: string | null | undefined;
} | null | undefined;
} | null | undefined;
};
export type RelayReaderNamedFragmentsTest2Query = {
response: RelayReaderNamedFragmentsTest2Query$data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ query Viewer($params: JSON) {
==================================== OUTPUT ===================================
import { JSON } from "TypeDefsFile";
export type Viewer$variables = {
params?: JSON | null;
params?: JSON | null | undefined;
};
export type Viewer$data = {
readonly viewer: {
readonly actor: {
readonly profilePicture2?: {
readonly __typename: "Image";
} | null;
} | null;
} | null;
} | null | undefined;
} | null | undefined;
} | null | undefined;
};
export type Viewer = {
response: Viewer$data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ export type ConcreateTypes$data = {
readonly " $fragmentSpreads": FragmentRefs<"PageFragment">;
} | {
readonly __typename: "User";
readonly name: string | null;
readonly name: string | null | undefined;
} | {
// This will never be '%other', but we need some
// value in case none of the concrete values match.
readonly __typename: "%other";
} | null;
} | null | undefined;
readonly " $fragmentType": "ConcreateTypes";
};
export type ConcreateTypes$key = {
Expand All @@ -72,13 +72,13 @@ export type ConcreateTypes$key = {
import { FragmentRefs } from "relay-runtime";
export type FragmentSpread$data = {
readonly fragAndField: {
readonly uri: string | null;
readonly uri: string | null | undefined;
readonly " $fragmentSpreads": FragmentRefs<"PictureFragment">;
} | null;
} | null | undefined;
readonly id: string;
readonly justFrag: {
readonly " $fragmentSpreads": FragmentRefs<"PictureFragment">;
} | null;
} | null | undefined;
readonly " $fragmentSpreads": FragmentRefs<"OtherFragment" | "UserFrag1" | "UserFrag2">;
readonly " $fragmentType": "FragmentSpread";
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ import { FragmentRefs } from "relay-runtime";
export type InlineFragment$data = {
readonly id: string;
readonly message?: {
readonly text: string | null;
} | null;
readonly name?: string | null;
readonly text: string | null | undefined;
} | null | undefined;
readonly name?: string | null | undefined;
readonly " $fragmentType": "InlineFragment";
};
export type InlineFragment$key = {
Expand All @@ -81,7 +81,7 @@ export type InlineFragment$key = {
import { FragmentRefs } from "relay-runtime";
export type InlineFragmentConditionalID$data = {
readonly id?: string;
readonly name?: string | null;
readonly name?: string | null | undefined;
readonly " $fragmentType": "InlineFragmentConditionalID";
};
export type InlineFragmentConditionalID$key = {
Expand All @@ -93,14 +93,14 @@ import { FragmentRefs } from "relay-runtime";
export type InlineFragmentKitchenSink$data = {
readonly actor: {
readonly id: string;
readonly name?: string | null;
readonly name?: string | null | undefined;
readonly profilePicture: {
readonly height?: number | null;
readonly uri: string | null;
readonly width?: number | null;
} | null;
readonly height?: number | null | undefined;
readonly uri: string | null | undefined;
readonly width?: number | null | undefined;
} | null | undefined;
readonly " $fragmentSpreads": FragmentRefs<"SomeFragment">;
} | null;
} | null | undefined;
readonly " $fragmentType": "InlineFragmentKitchenSink";
};
export type InlineFragmentKitchenSink$key = {
Expand All @@ -113,11 +113,11 @@ export type InlineFragmentWithOverlappingFields$data = {
readonly hometown?: {
readonly id: string;
readonly message?: {
readonly text: string | null;
} | null;
readonly name: string | null;
} | null;
readonly name?: string | null;
readonly text: string | null | undefined;
} | null | undefined;
readonly name: string | null | undefined;
} | null | undefined;
readonly name?: string | null | undefined;
readonly " $fragmentType": "InlineFragmentWithOverlappingFields";
};
export type InlineFragmentWithOverlappingFields$key = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export type UnionTypeTest$data = {
// This will never be '%other', but we need some
// value in case none of the concrete values match.
readonly __typename: "%other";
} | null;
} | null | undefined;
};
export type UnionTypeTest = {
response: UnionTypeTest$data;
Expand All @@ -47,18 +47,18 @@ import { FragmentRefs } from "relay-runtime";
export type LinkedField$data = {
readonly actor: {
readonly id: string;
} | null;
} | null | undefined;
readonly hometown: {
readonly id: string;
readonly profilePicture: {
readonly uri: string | null;
} | null;
} | null;
readonly uri: string | null | undefined;
} | null | undefined;
} | null | undefined;
readonly profilePicture: {
readonly height: number | null;
readonly uri: string | null;
readonly width: number | null;
} | null;
readonly height: number | null | undefined;
readonly uri: string | null | undefined;
readonly width: number | null | undefined;
} | null | undefined;
readonly " $fragmentType": "LinkedField";
};
export type LinkedField$key = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ export type NameRendererQuery$variables = {};
export type NameRendererQuery$data = {
readonly me: {
readonly nameRenderer: {
readonly __fragmentPropName?: string | null;
readonly __module_component?: string | null;
readonly __fragmentPropName?: string | null | undefined;
readonly __module_component?: string | null | undefined;
readonly " $fragmentSpreads": FragmentRefs<"MarkdownUserNameRenderer_name" | "PlainUserNameRenderer_name">;
} | null;
} | null;
} | null | undefined;
} | null | undefined;
};
export type NameRendererQuery = {
response: NameRendererQuery$data;
Expand All @@ -42,9 +42,9 @@ export type NameRendererQuery = {
import { FragmentRefs } from "relay-runtime";
export type MarkdownUserNameRenderer_name$data = {
readonly data: {
readonly markup: string | null;
} | null;
readonly markdown: string | null;
readonly markup: string | null | undefined;
} | null | undefined;
readonly markdown: string | null | undefined;
readonly " $fragmentType": "MarkdownUserNameRenderer_name";
};
export type MarkdownUserNameRenderer_name$key = {
Expand All @@ -55,9 +55,9 @@ export type MarkdownUserNameRenderer_name$key = {
import { FragmentRefs } from "relay-runtime";
export type PlainUserNameRenderer_name$data = {
readonly data: {
readonly text: string | null;
} | null;
readonly plaintext: string | null;
readonly text: string | null | undefined;
} | null | undefined;
readonly plaintext: string | null | undefined;
readonly " $fragmentType": "PlainUserNameRenderer_name";
};
export type PlainUserNameRenderer_name$key = {
Expand Down
Loading

0 comments on commit cc47bc9

Please sign in to comment.