Skip to content

Commit

Permalink
Take advantage of @semanticNonNull within @catch (#4794)
Browse files Browse the repository at this point in the history
Summary:
For fields with `catch(to: RESULT)` and fields nested within any `catch` we are handling all errors which might have caused a field to become null. This means we can respect semantic non-null in these positions.

Pull Request resolved: #4794

Reviewed By: gordyf

Differential Revision: D62645500

Pulled By: captbaritone

fbshipit-source-id: b2e3e80fcfd2a692fd942ce856fc42dbc02f4e77
  • Loading branch information
captbaritone authored and facebook-github-bot committed Sep 13, 2024
1 parent 79840ae commit 26b78db
Show file tree
Hide file tree
Showing 10 changed files with 248 additions and 44 deletions.
98 changes: 55 additions & 43 deletions compiler/crates/relay-typegen/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ use relay_transforms::RequiredMetadataDirective;
use relay_transforms::ResolverOutputTypeInfo;
use relay_transforms::TypeConditionInfo;
use relay_transforms::ASSIGNABLE_DIRECTIVE_FOR_TYPEGEN;
use relay_transforms::CATCH_DIRECTIVE_NAME;
use relay_transforms::CHILDREN_CAN_BUBBLE_METADATA_KEY;
use relay_transforms::CLIENT_EXTENSION_DIRECTIVE_NAME;
use relay_transforms::RELAY_ACTOR_CHANGE_DIRECTIVE_FOR_CODEGEN;
Expand Down Expand Up @@ -159,7 +160,7 @@ pub(crate) fn visit_selections(
runtime_imports: &mut RuntimeImports,
custom_error_import: &mut Option<CustomTypeImport>,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) -> Vec<TypeSelection> {
let mut type_selections = Vec::new();
for selection in selections {
Expand All @@ -175,7 +176,7 @@ pub(crate) fn visit_selections(
encountered_fragments,
imported_resolvers,
runtime_imports,
is_throw_on_field_error,
emit_semantic_types,
),
Selection::InlineFragment(inline_fragment) => visit_inline_fragment(
typegen_context,
Expand All @@ -191,7 +192,7 @@ pub(crate) fn visit_selections(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
),
Selection::LinkedField(linked_field) => {
let linked_field_type = typegen_context
Expand All @@ -205,6 +206,11 @@ pub(crate) fn visit_selections(
} else {
Some(linked_field_type)
};
let field_emit_semantic_types = emit_semantic_types
|| linked_field
.directives
.named(*CATCH_DIRECTIVE_NAME)
.is_some();
gen_visit_linked_field(
typegen_context,
&mut type_selections,
Expand All @@ -223,10 +229,10 @@ pub(crate) fn visit_selections(
runtime_imports,
custom_error_import,
nested_enclosing_linked_field_concrete_type,
is_throw_on_field_error,
field_emit_semantic_types,
)
},
is_throw_on_field_error,
emit_semantic_types,
)
}
Selection::ScalarField(scalar_field) => {
Expand All @@ -246,7 +252,7 @@ pub(crate) fn visit_selections(
resolver_metadata,
RequiredMetadataDirective::find(&scalar_field.directives).is_some(),
imported_resolvers,
is_throw_on_field_error,
emit_semantic_types,
);
} else {
visit_scalar_field(
Expand All @@ -256,7 +262,7 @@ pub(crate) fn visit_selections(
encountered_enums,
custom_scalars,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
)
}
}
Expand All @@ -274,7 +280,7 @@ pub(crate) fn visit_selections(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
),
}
}
Expand All @@ -293,7 +299,7 @@ fn visit_fragment_spread(
encountered_fragments: &mut EncounteredFragments,
imported_resolvers: &mut ImportedResolvers,
runtime_imports: &mut RuntimeImports,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
if let Some(resolver_metadata) = RelayResolverMetadata::find(&fragment_spread.directives) {
visit_relay_resolver(
Expand All @@ -309,7 +315,7 @@ fn visit_fragment_spread(
resolver_metadata,
RequiredMetadataDirective::find(&fragment_spread.directives).is_some(),
imported_resolvers,
is_throw_on_field_error,
emit_semantic_types,
);
} else {
let name = fragment_spread.fragment.item;
Expand Down Expand Up @@ -685,7 +691,7 @@ fn relay_resolver_field_type(
local_resolver_name: StringKey,
required: bool,
live: bool,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) -> AST {
let maybe_scalar_field =
if let ResolverOutputTypeInfo::ScalarField = resolver_metadata.output_type_info {
Expand All @@ -703,7 +709,7 @@ fn relay_resolver_field_type(
};

if let Some(field) = maybe_scalar_field {
let type_ = match is_throw_on_field_error {
let type_ = match emit_semantic_types {
true => field.semantic_type(),
false => field.type_.clone(),
};
Expand All @@ -721,7 +727,7 @@ fn relay_resolver_field_type(
}
} else {
let field = resolver_metadata.field(typegen_context.schema);
let field_type = match is_throw_on_field_error {
let field_type = match emit_semantic_types {
true => field.semantic_type(),
false => field.type_.clone(),
};
Expand Down Expand Up @@ -755,7 +761,7 @@ fn visit_relay_resolver(
resolver_metadata: &RelayResolverMetadata,
required: bool,
imported_resolvers: &mut ImportedResolvers,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
import_relay_resolver_function_type(
typegen_context,
Expand Down Expand Up @@ -785,7 +791,7 @@ fn visit_relay_resolver(
local_resolver_name,
required,
live,
is_throw_on_field_error,
emit_semantic_types,
);

type_selections.push(TypeSelection::ScalarField(TypeSelectionScalarField {
Expand Down Expand Up @@ -813,7 +819,7 @@ fn visit_client_edge(
runtime_imports: &mut RuntimeImports,
custom_error_import: &mut Option<CustomTypeImport>,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
let (resolver_metadata, fragment_name) = match &client_edge_metadata.backing_field {
Selection::FragmentSpread(fragment_spread) => (
Expand Down Expand Up @@ -858,7 +864,7 @@ fn visit_client_edge(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
);
type_selections.append(&mut client_edge_selections);
}
Expand All @@ -878,7 +884,7 @@ fn visit_inline_fragment(
runtime_imports: &mut RuntimeImports,
custom_error_import: &mut Option<CustomTypeImport>,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
if let Some(module_metadata) = ModuleMetadata::find(&inline_fragment.directives) {
let name = module_metadata.fragment_name;
Expand Down Expand Up @@ -925,7 +931,7 @@ fn visit_inline_fragment(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
);
} else if let Some(client_edge_metadata) = ClientEdgeMetadata::find(inline_fragment) {
visit_client_edge(
Expand All @@ -942,7 +948,7 @@ fn visit_inline_fragment(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
);
} else {
let mut inline_selections = visit_selections(
Expand Down Expand Up @@ -1028,7 +1034,7 @@ fn visit_actor_change(
runtime_imports: &mut RuntimeImports,
custom_error_import: &mut Option<CustomTypeImport>,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
let linked_field = match &inline_fragment.selections[0] {
Selection::LinkedField(linked_field) => linked_field,
Expand Down Expand Up @@ -1059,7 +1065,7 @@ fn visit_actor_change(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
);
type_selections.push(TypeSelection::ScalarField(TypeSelectionScalarField {
field_name_or_alias: key,
Expand Down Expand Up @@ -1098,7 +1104,7 @@ fn raw_response_visit_inline_fragment(
runtime_imports: &mut RuntimeImports,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
let mut selections = raw_response_visit_selections(
typegen_context,
Expand All @@ -1110,7 +1116,7 @@ fn raw_response_visit_inline_fragment(
runtime_imports,
custom_scalars,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
);
if inline_fragment
.directives
Expand Down Expand Up @@ -1161,7 +1167,7 @@ fn gen_visit_linked_field(
type_selections: &mut Vec<TypeSelection>,
linked_field: &LinkedField,
mut visit_selections_fn: impl FnMut(&[Selection]) -> Vec<TypeSelection>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
let field = typegen_context.schema.field(linked_field.definition.item);
let schema_name = field.name.item;
Expand All @@ -1174,13 +1180,16 @@ fn gen_visit_linked_field(

let coerce_to_nullable = has_explicit_catch_to_null(&linked_field.directives);

let node_type = match is_throw_on_field_error {
true => apply_directive_nullability(field, &linked_field.directives, coerce_to_nullable),
false => apply_required_directive_nullability(
let is_result_type = is_result_type_directive(&linked_field.directives);

let node_type = if emit_semantic_types || is_result_type {
apply_directive_nullability(field, &linked_field.directives, coerce_to_nullable)
} else {
apply_required_directive_nullability(
&field.type_,
&linked_field.directives,
coerce_to_nullable,
),
)
};

type_selections.push(TypeSelection::LinkedField(TypeSelectionLinkedField {
Expand All @@ -1189,7 +1198,7 @@ fn gen_visit_linked_field(
node_selections: selections_to_map(selections.into_iter(), true),
conditional: false,
concrete_type: None,
is_result_type: is_result_type_directive(&linked_field.directives),
is_result_type,
}));
}

Expand All @@ -1200,7 +1209,7 @@ fn visit_scalar_field(
encountered_enums: &mut EncounteredEnums,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
let field = typegen_context.schema.field(scalar_field.definition.item);
let schema_name = field.name.item;
Expand All @@ -1212,13 +1221,16 @@ fn visit_scalar_field(

let coerce_to_nullable = has_explicit_catch_to_null(&scalar_field.directives);

let field_type = match is_throw_on_field_error {
true => apply_directive_nullability(field, &scalar_field.directives, coerce_to_nullable),
false => apply_required_directive_nullability(
let is_result_type = is_result_type_directive(&scalar_field.directives);

let field_type = if emit_semantic_types || is_result_type {
apply_directive_nullability(field, &scalar_field.directives, coerce_to_nullable)
} else {
apply_required_directive_nullability(
&field.type_,
&scalar_field.directives,
coerce_to_nullable,
),
)
};

let special_field = ScalarFieldSpecialSchemaField::from_schema_name(
Expand Down Expand Up @@ -1262,7 +1274,7 @@ fn visit_scalar_field(
value: ast,
conditional: false,
concrete_type: None,
is_result_type: is_result_type_directive(&scalar_field.directives),
is_result_type,
}));
}

Expand All @@ -1281,7 +1293,7 @@ fn visit_condition(
runtime_imports: &mut RuntimeImports,
custom_error_import: &mut Option<CustomTypeImport>,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) {
let mut selections = visit_selections(
typegen_context,
Expand All @@ -1296,7 +1308,7 @@ fn visit_condition(
runtime_imports,
custom_error_import,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
);
for selection in selections.iter_mut() {
selection.set_conditional(true);
Expand Down Expand Up @@ -2218,7 +2230,7 @@ pub(crate) fn raw_response_visit_selections(
runtime_imports: &mut RuntimeImports,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
is_throw_on_field_error: bool,
emit_semantic_types: bool,
) -> Vec<TypeSelection> {
let mut type_selections = Vec::new();
for selection in selections {
Expand Down Expand Up @@ -2294,10 +2306,10 @@ pub(crate) fn raw_response_visit_selections(
runtime_imports,
custom_scalars,
nested_enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
)
},
is_throw_on_field_error,
emit_semantic_types,
)
}
Selection::ScalarField(scalar_field) => visit_scalar_field(
Expand All @@ -2307,7 +2319,7 @@ pub(crate) fn raw_response_visit_selections(
encountered_enums,
custom_scalars,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
),
Selection::Condition(condition) => {
type_selections.extend(raw_response_visit_selections(
Expand All @@ -2320,7 +2332,7 @@ pub(crate) fn raw_response_visit_selections(
runtime_imports,
custom_scalars,
enclosing_linked_field_concrete_type,
is_throw_on_field_error,
emit_semantic_types,
));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
==================================== INPUT ====================================
fragment MyFragment on Query {
# Should be a Result of a non-nullable since it's semanticNonNull
my_user @catch {
name
}
}

%extensions%

extend type Query {
my_user: User @semanticNonNull
}
==================================== OUTPUT ===================================
import { FragmentRefs, Result } from "relay-runtime";
export type MyFragment$data = {
readonly my_user: Result<{
readonly name: string | null | undefined;
}, ReadonlyArray<unknown>>;
readonly " $fragmentType": "MyFragment";
};
export type MyFragment$key = {
readonly " $data"?: MyFragment$data;
readonly " $fragmentSpreads": FragmentRefs<"MyFragment">;
};
Loading

0 comments on commit 26b78db

Please sign in to comment.