Skip to content

Commit

Permalink
Make __typename selections within concrete linked fields have a strin…
Browse files Browse the repository at this point in the history 
…g literal flowtype

Summary:
# What and why

* Currently, a selection like `foo { __typename }` will generate a type of `__typename: string`. We have never had a need to generate a more accurate type, hence we generate `string` instead of a string literal for the value of `__typename`.
* However, this is problematic for typesafe updaters! This requires assignments of `foo` to be validated, **even though we known statically that the validation can never fail**.
* With this change, assigning an array of linked fields is modified as follows:

```
const data = useFragment(graphql`fragment SourceFragment on User {
  best_friends(count: 5) required(action: THROW) {
    ...Assignable_user
  }
}`, userRef);

const env = useRelayEnvironment();
const onClick = () => {
  env.commitUpdate(store => {
    const {updatableData} = store.readUpdatableQuery(graphql`
      query Updatable_best_friends updatable {
        best_friends {
          ...Assignable_user
        }
      }
    `, {});

    // WE CAN REPLACE THIS
    const validateUser = require('SourceFragment').validate;
    const validBestFriends = data.best_friends.flatMap(bestFriend => {
      const validBestFriend = validateUser(bestFriend);
      if (validBestFriend !== false) {
        return [validBestFriend];
      } else {
        // this case never occurs!
        return [];
      }
    });
    updatableData.best_friends = validBestFriends;

    // WITH THIS
    updatableData.best_friends = data.best_friends
  });
}
```

# How

Pass around a `enclosing_concrete_linked_field_type: Option<Type>` through the codegen. Whenever we visit a linked field, we potentially pass `Some(linked_field_type)`. When we process a scalar field, if that parameter is Some, then we generate an AST with the StringLiteral of tha type.

# Rollout

This needs to rollout with a rollout. Once this diff is approved, I'll put a rollout in.

Reviewed By: alunyov

Differential Revision: D36357062

fbshipit-source-id: e06d076e6395183eb7872e47962c81e6cd4db475
  • Loading branch information
@rbalicki2 @facebook-github-bot
rbalicki2 authored and facebook-github-bot committed Jun 3, 2022
1 parent 90b4226 commit 20626f2
Show file tree
Hide file tree
Showing 24 changed files with 183 additions and 75 deletions.
15 changes: 15 additions & 0 deletions compiler/crates/relay-compiler/src/file_source/file_categorizer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,23 +374,38 @@ mod tests {
},
"projects": {
"public": {
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"schema": "graphql/public.graphql",
"language": "flow"
},
"internal": {
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"schema": "graphql/__generated__/internal.graphql",
"language": "flow"
},
"with_custom_generated_dir": {
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"schema": "graphql/__generated__/custom.graphql",
"output": "graphql/custom-generated",
"language": "flow"
},
"typescript": {
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"schema": "graphql/ts_schema.graphql",
"language": "typescript"
},
"overlapping_generated_dir": {
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"schema": "graphql/__generated__/custom.graphql",
"language": "flow"
}
Expand Down
3 changes: 3 additions & 0 deletions ...compile_relay_artifacts/fixtures/refetchable-fragment-with-connection-es-modules.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ fragment refetchableFragmentWithConnectionEsModules_PaginationFragment on Node

%project_config%
{
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"eagerEsModules": true,
"language": "flow"
}
Expand Down
3 changes: 3 additions & 0 deletions .../compile_relay_artifacts/fixtures/refetchable-fragment-with-connection-es-modules.graphql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ fragment refetchableFragmentWithConnectionEsModules_PaginationFragment on Node

%project_config%
{
"preciseTypenameTypesWithinLinkedFields": {
"kind": "enabled"
},
"eagerEsModules": true,
"language": "flow"
}
4 changes: 4 additions & 0 deletions compiler/crates/relay-config/src/typegen_config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

use common::FeatureFlag;
use fnv::FnvBuildHasher;
use indexmap::IndexMap;
use intern::string_key::StringKey;
Expand Down Expand Up @@ -87,6 +88,9 @@ pub struct TypegenConfig {
#[serde(default)]
pub use_import_type_syntax: bool,

/// This feature is used to rollout precise __typename type generation.
pub precise_typename_types_within_linked_fields: FeatureFlag,

/// A map from GraphQL scalar types to a custom JS type, example:
/// { "Url": "String" }
/// { "Url": {"name:: "MyURL", "path": "../src/MyUrlTypes"} }
Expand Down
153 changes: 116 additions & 37 deletions compiler/crates/relay-typegen/src/visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub(crate) fn visit_selections(
imported_resolvers: &mut ImportedResolvers,
actor_change_status: &mut ActorChangeStatus,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) -> Vec<TypeSelection> {
let mut type_selections = Vec::new();
for selection in selections {
Expand All @@ -74,29 +75,45 @@ pub(crate) fn visit_selections(
imported_resolvers,
actor_change_status,
custom_scalars,
enclosing_linked_field_concrete_type,
),
Selection::LinkedField(linked_field) => gen_visit_linked_field(
typegen_context.schema,
&mut type_selections,
linked_field,
|selections| {
visit_selections(
typegen_context,
selections,
encountered_enums,
encountered_fragments,
imported_resolvers,
actor_change_status,
custom_scalars,
)
},
),
Selection::LinkedField(linked_field) => {
let linked_field_type = typegen_context
.schema
.field(linked_field.definition.item)
.type_
.inner();
let nested_enclosing_linked_field_concrete_type =
if linked_field_type.is_abstract_type() {
None
} else {
Some(linked_field_type)
};
gen_visit_linked_field(
typegen_context.schema,
&mut type_selections,
linked_field,
|selections| {
visit_selections(
typegen_context,
selections,
encountered_enums,
encountered_fragments,
imported_resolvers,
actor_change_status,
custom_scalars,
nested_enclosing_linked_field_concrete_type,
)
},
)
}
Selection::ScalarField(scalar_field) => visit_scalar_field(
typegen_context,
&mut type_selections,
scalar_field,
encountered_enums,
custom_scalars,
enclosing_linked_field_concrete_type,
),
Selection::Condition(condition) => visit_condition(
typegen_context,
Expand All @@ -107,6 +124,7 @@ pub(crate) fn visit_selections(
imported_resolvers,
actor_change_status,
custom_scalars,
enclosing_linked_field_concrete_type,
),
}
}
Expand Down Expand Up @@ -224,6 +242,7 @@ fn visit_inline_fragment(
imported_resolvers: &mut ImportedResolvers,
actor_change_status: &mut ActorChangeStatus,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) {
if let Some(module_metadata) = ModuleMetadata::find(&inline_fragment.directives) {
let name = module_metadata.fragment_name;
Expand Down Expand Up @@ -261,6 +280,7 @@ fn visit_inline_fragment(
imported_resolvers,
actor_change_status,
custom_scalars,
enclosing_linked_field_concrete_type,
);
} else {
let mut inline_selections = visit_selections(
Expand All @@ -271,6 +291,7 @@ fn visit_inline_fragment(
imported_resolvers,
actor_change_status,
custom_scalars,
enclosing_linked_field_concrete_type,
);

let mut selections = if let Some(fragment_alias_metadata) =
Expand Down Expand Up @@ -329,6 +350,7 @@ fn visit_actor_change(
imported_resolvers: &mut ImportedResolvers,
actor_change_status: &mut ActorChangeStatus,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) {
let linked_field = match &inline_fragment.selections[0] {
Selection::LinkedField(linked_field) => linked_field,
Expand All @@ -354,6 +376,7 @@ fn visit_actor_change(
imported_resolvers,
actor_change_status,
custom_scalars,
enclosing_linked_field_concrete_type,
);
type_selections.push(TypeSelection::ScalarField(TypeSelectionScalarField {
field_name_or_alias: key,
Expand Down Expand Up @@ -388,6 +411,7 @@ fn raw_response_visit_inline_fragment(
imported_raw_response_types: &mut ImportedRawResponseTypes,
runtime_imports: &mut RuntimeImports,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) {
let mut selections = raw_response_visit_selections(
typegen_context,
Expand All @@ -398,6 +422,7 @@ fn raw_response_visit_inline_fragment(
imported_raw_response_types,
runtime_imports,
custom_scalars,
enclosing_linked_field_concrete_type,
);
if inline_fragment
.directives
Expand Down Expand Up @@ -475,6 +500,7 @@ fn visit_scalar_field(
scalar_field: &ScalarField,
encountered_enums: &mut EncounteredEnums,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) {
let field = typegen_context.schema.field(scalar_field.definition.item);
let schema_name = field.name.item;
Expand All @@ -484,12 +510,45 @@ fn visit_scalar_field(
schema_name
};
let field_type = apply_required_directive_nullability(&field.type_, &scalar_field.directives);
let special_field = ScalarFieldSpecialSchemaField::from_schema_name(
schema_name,
&typegen_context.project_config.schema_config,
);

if typegen_context
.project_config
.typegen_config
.precise_typename_types_within_linked_fields
.is_enabled_for(typegen_context.definition_source_location.item)
&& matches!(special_field, Some(ScalarFieldSpecialSchemaField::TypeName))
{
if let Some(concrete_type) = enclosing_linked_field_concrete_type {
// If we are creating a typename selection within a linked field with a concrete type, we generate
// the type e.g. "User", i.e. the concrete string name of the concrete type.
//
// This cannot be done within abstract fields and at the top level (even in fragments), because
// we have the following type hole. With `node { ...Fragment_user }`, `Fragment_user` can be
// unconditionally read out, without checking whether the `node` field actually has a matching
// type at runtime.
//
// Note that passing concrete_type: enclosing_linked_field_concrete_type here has the effect
// of making the emitted fields left-hand-optional, causing the compiler to panic (because
// within updatable fragments/queries, we expect never to generate an optional type.)
return type_selections.push(TypeSelection::ScalarField(TypeSelectionScalarField {
field_name_or_alias: key,
special_field,
value: AST::StringLiteral(StringLiteral(
typegen_context.schema.get_type_name(concrete_type),
)),
conditional: false,
concrete_type: None,
}));
}
}

type_selections.push(TypeSelection::ScalarField(TypeSelectionScalarField {
field_name_or_alias: key,
special_field: ScalarFieldSpecialSchemaField::from_schema_name(
schema_name,
&typegen_context.project_config.schema_config,
),
special_field,
value: transform_scalar_type(
typegen_context,
&field_type,
Expand All @@ -512,6 +571,7 @@ fn visit_condition(
imported_resolvers: &mut ImportedResolvers,
actor_change_status: &mut ActorChangeStatus,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) {
let mut selections = visit_selections(
typegen_context,
Expand All @@ -521,6 +581,7 @@ fn visit_condition(
imported_resolvers,
actor_change_status,
custom_scalars,
enclosing_linked_field_concrete_type,
);
for selection in selections.iter_mut() {
selection.set_conditional(true);
Expand Down Expand Up @@ -1312,6 +1373,7 @@ pub(crate) fn raw_response_visit_selections(
imported_raw_response_types: &mut ImportedRawResponseTypes,
runtime_imports: &mut RuntimeImports,
custom_scalars: &mut CustomScalarsImports,
enclosing_linked_field_concrete_type: Option<Type>,
) -> Vec<TypeSelection> {
let mut type_selections = Vec::new();
for selection in selections {
Expand Down Expand Up @@ -1341,30 +1403,46 @@ pub(crate) fn raw_response_visit_selections(
imported_raw_response_types,
runtime_imports,
custom_scalars,
enclosing_linked_field_concrete_type,
),
Selection::LinkedField(linked_field) => gen_visit_linked_field(
typegen_context.schema,
&mut type_selections,
linked_field,
|selections| {
raw_response_visit_selections(
typegen_context,
selections,
encountered_enums,
match_fields,
encountered_fragments,
imported_raw_response_types,
runtime_imports,
custom_scalars,
)
},
),
Selection::LinkedField(linked_field) => {
let linked_field_type = typegen_context
.schema
.field(linked_field.definition.item)
.type_
.inner();
let nested_enclosing_linked_field_concrete_type =
if linked_field_type.is_abstract_type() {
None
} else {
Some(linked_field_type)
};
gen_visit_linked_field(
typegen_context.schema,
&mut type_selections,
linked_field,
|selections| {
raw_response_visit_selections(
typegen_context,
selections,
encountered_enums,
match_fields,
encountered_fragments,
imported_raw_response_types,
runtime_imports,
custom_scalars,
nested_enclosing_linked_field_concrete_type,
)
},
)
}
Selection::ScalarField(scalar_field) => visit_scalar_field(
typegen_context,
&mut type_selections,
scalar_field,
encountered_enums,
custom_scalars,
enclosing_linked_field_concrete_type,
),
Selection::Condition(condition) => {
type_selections.extend(raw_response_visit_selections(
Expand All @@ -1376,6 +1454,7 @@ pub(crate) fn raw_response_visit_selections(
imported_raw_response_types,
runtime_imports,
custom_scalars,
enclosing_linked_field_concrete_type,
));
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/crates/relay-typegen/src/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub(crate) fn write_operation_type_exports_section(
&mut imported_resolvers,
&mut actor_change_status,
&mut custom_scalars,
None,
);

let mut imported_raw_response_types = Default::default();
Expand Down Expand Up @@ -94,6 +95,7 @@ pub(crate) fn write_operation_type_exports_section(
&mut imported_raw_response_types,
&mut runtime_imports,
&mut custom_scalars,
None,
);
Some((
raw_response_selections_to_babel(
Expand Down Expand Up @@ -229,6 +231,7 @@ pub(crate) fn write_split_operation_type_exports_section(
&mut imported_raw_response_types,
&mut runtime_imports,
&mut custom_scalars,
None,
);
let raw_response_type = raw_response_selections_to_babel(
typgen_context,
Expand Down Expand Up @@ -279,6 +282,7 @@ pub(crate) fn write_fragment_type_exports_section(
&mut imported_resolvers,
&mut actor_change_status,
&mut custom_scalars,
None,
);
if !fragment_definition.type_condition.is_abstract_type() {
let num_concrete_selections = type_selections
Expand Down
Loading

0 comments on commit 20626f2

Please sign in to comment.