diff --git a/compiler/crates/common/src/feature_flags.rs b/compiler/crates/common/src/feature_flags.rs index ac67d37726e84..0ae2c76abdea8 100644 --- a/compiler/crates/common/src/feature_flags.rs +++ b/compiler/crates/common/src/feature_flags.rs @@ -84,12 +84,27 @@ pub struct FeatureFlags { /// Perform strict validations when custom scalar types are used #[serde(default)] pub enable_strict_custom_scalars: bool, + + /// Relay Resolvers are a read-time feature that are not actually handled in + /// our mutation APIs. We are in the process of removing any existing + /// examples, but this flag is part of a process of removing any existing + /// examples. + #[serde(default)] + pub allow_resolvers_in_mutation_response: FeatureFlag, + + /// @required with an action of THROW is read-time feature that is not + /// compatible with our mutation APIs. We are in the process of removing + /// any existing examples, but this flag is part of a process of removing + /// any existing examples. + #[serde(default)] + pub allow_required_in_mutation_response: FeatureFlag, } -#[derive(Debug, Deserialize, Clone, Serialize)] +#[derive(Debug, Deserialize, Clone, Serialize, Default)] #[serde(tag = "kind", rename_all = "lowercase")] pub enum FeatureFlag { /// Fully disabled: developers may not use this feature + #[default] Disabled, /// Fully enabled: developers may use this feature @@ -102,12 +117,6 @@ pub enum FeatureFlag { Rollout { rollout: Rollout }, } -impl Default for FeatureFlag { - fn default() -> Self { - FeatureFlag::Disabled - } -} - impl FeatureFlag { pub fn is_enabled_for(&self, name: StringKey) -> bool { match self { diff --git a/compiler/crates/graphql-ir/src/errors.rs b/compiler/crates/graphql-ir/src/errors.rs index 2fcbe8921ad4d..998e3715ef3c0 100644 --- a/compiler/crates/graphql-ir/src/errors.rs +++ b/compiler/crates/graphql-ir/src/errors.rs @@ -516,6 +516,16 @@ pub enum ValidationMessage { literal_kind: String, scalar_type_name: ScalarName, }, + + #[error( + "Unexpected `@required(action: THROW)` directive in mutation response. The use of `@required(action: THROW)` is not supported in mutations." + )] + RequiredInMutation, + + #[error( + "Unexpected `@RelayResolver` field referenced in mutation response. Relay Resolver fields may not be read as part of a mutation response." + )] + ResolverInMutation, } #[derive(Clone, Debug, Error, Eq, PartialEq, Ord, PartialOrd, Hash)] diff --git a/compiler/crates/relay-compiler/src/build_project/validate.rs b/compiler/crates/relay-compiler/src/build_project/validate.rs index 3318af26c353c..bfe6a555a73df 100644 --- a/compiler/crates/relay-compiler/src/build_project/validate.rs +++ b/compiler/crates/relay-compiler/src/build_project/validate.rs @@ -15,6 +15,7 @@ use errors::try_all; use graphql_ir::Program; use relay_config::ProjectConfig; use relay_transforms::disallow_circular_no_inline_fragments; +use relay_transforms::disallow_readtime_features_in_mutations; use relay_transforms::disallow_reserved_aliases; use relay_transforms::disallow_typename_on_root; use relay_transforms::validate_assignable_directive; @@ -67,6 +68,16 @@ pub fn validate( } else { Ok(()) }, + disallow_readtime_features_in_mutations( + program, + &project_config + .feature_flags + .allow_resolvers_in_mutation_response, + &project_config + .feature_flags + .allow_required_in_mutation_response, + project_config.feature_flags.enable_relay_resolver_mutations, + ), ]); match output { diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs index f2d792c42d49c..866f73b804710 100644 --- a/compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs +++ b/compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs @@ -122,6 +122,8 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result relay_resolvers_allow_legacy_verbose_syntax: FeatureFlag::Disabled, enable_relay_resolver_mutations: false, enable_strict_custom_scalars: false, + allow_required_in_mutation_response: FeatureFlag::Disabled, + allow_resolvers_in_mutation_response: FeatureFlag::Disabled, }; let default_project_config = ProjectConfig { diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts_with_custom_id/mod.rs b/compiler/crates/relay-compiler/tests/compile_relay_artifacts_with_custom_id/mod.rs index 3f1e4cc15d2e8..8eae4bfc5791a 100644 --- a/compiler/crates/relay-compiler/tests/compile_relay_artifacts_with_custom_id/mod.rs +++ b/compiler/crates/relay-compiler/tests/compile_relay_artifacts_with_custom_id/mod.rs @@ -101,6 +101,8 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result relay_resolvers_allow_legacy_verbose_syntax: FeatureFlag::Disabled, enable_relay_resolver_mutations: false, enable_strict_custom_scalars: false, + allow_required_in_mutation_response: FeatureFlag::Disabled, + allow_resolvers_in_mutation_response: FeatureFlag::Disabled, }; let default_schema_config = SchemaConfig::default(); diff --git a/compiler/crates/relay-transforms/Cargo.toml b/compiler/crates/relay-transforms/Cargo.toml index 8a73c5f392389..45232ed5fb613 100644 --- a/compiler/crates/relay-transforms/Cargo.toml +++ b/compiler/crates/relay-transforms/Cargo.toml @@ -1,4 +1,4 @@ -# @generated by autocargo from //relay/oss/crates/relay-transforms:[apply_fragment_arguments_test,assignable_directive_test,assignable_fragment_spread_test,client_edges_test,client_extensions_test,declarative_connection_test,disallow_typename_on_root_test,fragment_alias_directive_test,generate_data_driven_dependency_metadata_test,generate_live_query_metadata_test,generate_relay_resolvers_operations_for_nested_objects_test,graphql-client_extensions_abstract_types-test,graphql-defer_stream-test,graphql-disallow_non_node_id_fields-test,graphql-disallow_reserved_aliases-test,graphql-flatten-test,graphql-generate_id_field-test,graphql-generate_typename-test,graphql-inline_fragments-test,graphql-mask-test,graphql-match-local-test,graphql-match-test,graphql-node_identifier-test,graphql-refetchable_fragment_test,graphql-skip_client_extensions-test,graphql-skip_redundant_nodes-test,graphql-skip_unreachable_nodes-test,graphql-sort_selections-test,graphql-subscription_transform-test,graphql-validate_deprecated_fields_test,graphql-validate_module_names-test,graphql-validate_relay_directives-test,graphql-validate_required_arguments_test,graphql-validate_server_only_directives-test,graphql-validate_unused_variables-test,inline_data_fragment_test,provided-variable-fragment-transform-test,relay-actor-change-test,relay-transforms,relay_resolvers_abstract_types_test,relay_resolvers_test,relay_test_operation_test,required_directive_test,skip_unused_variables_test,transform_connections_test,updatable_directive_test,updatable_fragment_spread_test,validate_connections_schema_test,validate_connections_test,validate_global_variable_names_test,validate_global_variables-test,validate_no_double_underscore_alias_test,validate_no_unselectable_selections_test,validate_static_args] +# @generated by autocargo from //relay/oss/crates/relay-transforms:[apply_fragment_arguments_test,assignable_directive_test,assignable_fragment_spread_test,client_edges_test,client_extensions_test,declarative_connection_test,disallow_typename_on_root_test,fragment_alias_directive_test,generate_data_driven_dependency_metadata_test,generate_live_query_metadata_test,generate_relay_resolvers_operations_for_nested_objects_test,graphql-client_extensions_abstract_types-test,graphql-defer_stream-test,graphql-disallow_non_node_id_fields-test,graphql-disallow_reserved_aliases-test,graphql-disallowreadtime_features_in_mutations-test,graphql-flatten-test,graphql-generate_id_field-test,graphql-generate_typename-test,graphql-inline_fragments-test,graphql-mask-test,graphql-match-local-test,graphql-match-test,graphql-node_identifier-test,graphql-refetchable_fragment_test,graphql-skip_client_extensions-test,graphql-skip_redundant_nodes-test,graphql-skip_unreachable_nodes-test,graphql-sort_selections-test,graphql-subscription_transform-test,graphql-validate_deprecated_fields_test,graphql-validate_module_names-test,graphql-validate_relay_directives-test,graphql-validate_required_arguments_test,graphql-validate_server_only_directives-test,graphql-validate_unused_variables-test,inline_data_fragment_test,provided-variable-fragment-transform-test,relay-actor-change-test,relay-transforms,relay_resolvers_abstract_types_test,relay_resolvers_test,relay_test_operation_test,required_directive_test,skip_unused_variables_test,transform_connections_test,updatable_directive_test,updatable_fragment_spread_test,validate_connections_schema_test,validate_connections_test,validate_global_variable_names_test,validate_global_variables-test,validate_no_double_underscore_alias_test,validate_no_unselectable_selections_test,validate_static_args] [package] name = "relay-transforms" @@ -24,6 +24,10 @@ path = "tests/disallow_non_node_id_fields_test.rs" name = "graphql_disallow_reserved_aliases_test" path = "tests/disallow_reserved_aliases_test.rs" +[[test]] +name = "graphql_disallowreadtime_features_in_mutations_test" +path = "tests/disallow_readtime_features_in_mutations_test.rs" + [[test]] name = "graphql_flatten_test" path = "tests/flatten_test.rs" diff --git a/compiler/crates/relay-transforms/src/lib.rs b/compiler/crates/relay-transforms/src/lib.rs index 418ac399b40c2..794a1f9ede3cd 100644 --- a/compiler/crates/relay-transforms/src/lib.rs +++ b/compiler/crates/relay-transforms/src/lib.rs @@ -179,6 +179,7 @@ pub use required_directive::RequiredMetadataDirective; pub use required_directive::ACTION_ARGUMENT; pub use required_directive::CHILDREN_CAN_BUBBLE_METADATA_KEY; pub use required_directive::REQUIRED_DIRECTIVE_NAME; +pub use required_directive::THROW_ACTION; pub use skip_client_directives::skip_client_directives; pub use skip_client_extensions::skip_client_extensions; pub use skip_null_arguments_transform::skip_null_arguments_transform; diff --git a/compiler/crates/relay-transforms/src/required_directive/mod.rs b/compiler/crates/relay-transforms/src/required_directive/mod.rs index 2c01cce3b4c78..3669dd39c70ae 100644 --- a/compiler/crates/relay-transforms/src/required_directive/mod.rs +++ b/compiler/crates/relay-transforms/src/required_directive/mod.rs @@ -50,7 +50,7 @@ lazy_static! { pub static ref ACTION_ARGUMENT: ArgumentName = ArgumentName("action".intern()); pub static ref CHILDREN_CAN_BUBBLE_METADATA_KEY: DirectiveName = DirectiveName("__childrenCanBubbleNull".intern()); - static ref THROW_ACTION: StringKey = "THROW".intern(); + pub static ref THROW_ACTION: StringKey = "THROW".intern(); static ref LOG_ACTION: StringKey = "LOG".intern(); static ref NONE_ACTION: StringKey = "NONE".intern(); static ref INLINE_DIRECTIVE_NAME: DirectiveName = DirectiveName("inline".intern()); diff --git a/compiler/crates/relay-transforms/src/validations/disallow_readtime_features_in_mutations.rs b/compiler/crates/relay-transforms/src/validations/disallow_readtime_features_in_mutations.rs new file mode 100644 index 0000000000000..795cdb250fff4 --- /dev/null +++ b/compiler/crates/relay-transforms/src/validations/disallow_readtime_features_in_mutations.rs @@ -0,0 +1,153 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +use common::Diagnostic; +use common::DiagnosticsResult; +use common::FeatureFlag; +use common::NamedItem; +use docblock_shared::RELAY_RESOLVER_DIRECTIVE_NAME; +use graphql_ir::ConstantValue; +use graphql_ir::Field; +use graphql_ir::FragmentDefinition; +use graphql_ir::FragmentSpread; +use graphql_ir::LinkedField; +use graphql_ir::OperationDefinition; +use graphql_ir::Program; +use graphql_ir::ScalarField; +use graphql_ir::ValidationMessage; +use graphql_ir::Validator; +use schema::Schema; + +use crate::ACTION_ARGUMENT; +use crate::REQUIRED_DIRECTIVE_NAME; +use crate::THROW_ACTION; + +/// Some Relay features will cause a field to throw or suspend at read time. +/// These behaviors are incompatible with our mutation APIs. +/// This validator checks that no such features are used in mutations. +pub fn disallow_readtime_features_in_mutations( + program: &Program, + allow_resolvers_mutation_response: &FeatureFlag, + allow_required_in_mutation_response: &FeatureFlag, + enable_relay_resolver_mutations: bool, +) -> DiagnosticsResult<()> { + let mut validator = DisallowReadtimeFeaturesInMutations::new( + program, + allow_resolvers_mutation_response.clone(), + allow_required_in_mutation_response.clone(), + enable_relay_resolver_mutations, + ); + validator.validate_program(program) +} + +struct DisallowReadtimeFeaturesInMutations<'program> { + program: &'program Program, + allow_resolvers_mutation_response: FeatureFlag, + allow_required_in_mutation_response: FeatureFlag, + enable_relay_resolver_mutations: bool, + allow_resolvers_for_this_mutation: bool, + allow_required_for_this_mutation: bool, +} + +impl<'program> DisallowReadtimeFeaturesInMutations<'program> { + fn new( + program: &'program Program, + allow_resolvers_mutation_response: FeatureFlag, + allow_required_in_mutation_response: FeatureFlag, + enable_relay_resolver_mutations: bool, + ) -> Self { + Self { + program, + allow_resolvers_mutation_response, + allow_required_in_mutation_response, + enable_relay_resolver_mutations, + allow_resolvers_for_this_mutation: false, + allow_required_for_this_mutation: false, + } + } + + fn validate_field(&self, field: &impl Field) -> DiagnosticsResult<()> { + if !self.allow_required_for_this_mutation { + if let Some(directive) = field.directives().named(*REQUIRED_DIRECTIVE_NAME) { + let action = directive + .arguments + .named(*ACTION_ARGUMENT) + .and_then(|arg| arg.value.item.get_constant()); + if let Some(ConstantValue::Enum(action)) = action { + if *action == *THROW_ACTION { + return Err(vec![Diagnostic::error( + ValidationMessage::RequiredInMutation, + directive.name.location, + )]); + } + } + } + } + if !self.allow_resolvers_for_this_mutation + && self + .program + .schema + .field(field.definition().item) + .directives + .named(*RELAY_RESOLVER_DIRECTIVE_NAME) + .is_some() + { + return Err(vec![Diagnostic::error( + ValidationMessage::ResolverInMutation, + field.alias_or_name_location(), + )]); + } + + Ok(()) + } +} + +impl Validator for DisallowReadtimeFeaturesInMutations<'_> { + const NAME: &'static str = "disallow_readtime_features_in_mutations"; + const VALIDATE_ARGUMENTS: bool = false; + const VALIDATE_DIRECTIVES: bool = false; + + fn validate_operation(&mut self, operation: &OperationDefinition) -> DiagnosticsResult<()> { + if !operation.is_mutation() { + // No need to traverse into non-mutation operations + return Ok(()); + } + self.allow_resolvers_for_this_mutation = self.enable_relay_resolver_mutations + || self + .allow_resolvers_mutation_response + .is_enabled_for(operation.name.item.0); + self.allow_required_for_this_mutation = self + .allow_required_in_mutation_response + .is_enabled_for(operation.name.item.0); + let result = self.default_validate_operation(operation); + + // Reset state + self.allow_resolvers_for_this_mutation = false; + self.allow_required_for_this_mutation = false; + + result + } + + fn validate_fragment(&mut self, _fragment: &FragmentDefinition) -> DiagnosticsResult<()> { + // We only care about mutations + Ok(()) + } + + fn validate_fragment_spread(&mut self, _spread: &FragmentSpread) -> DiagnosticsResult<()> { + // Values nested within fragment spreads are fine since they are not read as part of the + // mutation response. + Ok(()) + } + + fn validate_scalar_field(&mut self, field: &ScalarField) -> DiagnosticsResult<()> { + self.validate_field(field) + } + fn validate_linked_field(&mut self, field: &LinkedField) -> DiagnosticsResult<()> { + self.validate_field(field)?; + self.default_validate_linked_field(field) + } +} diff --git a/compiler/crates/relay-transforms/src/validations/mod.rs b/compiler/crates/relay-transforms/src/validations/mod.rs index 0d7e3fd06feb7..76db6650448ad 100644 --- a/compiler/crates/relay-transforms/src/validations/mod.rs +++ b/compiler/crates/relay-transforms/src/validations/mod.rs @@ -8,6 +8,7 @@ mod deprecated_fields; mod disallow_circular_no_inline_fragments; mod disallow_non_node_id_fields; +mod disallow_readtime_features_in_mutations; mod disallow_reserved_aliases; mod disallow_typename_on_root; mod validate_connections; @@ -29,6 +30,7 @@ pub use deprecated_fields::deprecated_fields; pub use deprecated_fields::deprecated_fields_for_executable_definition; pub use disallow_circular_no_inline_fragments::disallow_circular_no_inline_fragments; pub use disallow_non_node_id_fields::disallow_non_node_id_fields; +pub use disallow_readtime_features_in_mutations::disallow_readtime_features_in_mutations; pub use disallow_reserved_aliases::disallow_reserved_aliases; pub use disallow_typename_on_root::disallow_typename_on_root; pub use validate_connections::validate_connections; diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.expected new file mode 100644 index 0000000000000..0bdbbd81a4e42 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.expected @@ -0,0 +1,14 @@ +==================================== INPUT ==================================== +mutation MyMutation { + setName(name: "Alice") { + ...myFragment + } +} + +fragment myFragment on User { + name @required(action: THROW) +} + +# %extensions% +==================================== OUTPUT =================================== +OK diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.graphql new file mode 100644 index 0000000000000..95559422408e0 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.graphql @@ -0,0 +1,11 @@ +mutation MyMutation { + setName(name: "Alice") { + ...myFragment + } +} + +fragment myFragment on User { + name @required(action: THROW) +} + +# %extensions% diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.expected new file mode 100644 index 0000000000000..10fbfe1483c2a --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.expected @@ -0,0 +1,21 @@ +==================================== INPUT ==================================== +# expected-to-throw +mutation MyMutation { + some_resolver +} + +# %extensions% +type SomeType { + id: ID +} +extend type Mutation { + some_resolver: SomeType @relay_resolver +} +==================================== ERROR ==================================== +✖︎ Expected selections on field `some_resolver` of type `Mutation` + + mutation_with_linked_resolver.invalid.graphql:3:3 + 2 │ mutation MyMutation { + 3 │ some_resolver + │ ^^^^^^^^^^^^^ + 4 │ } diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.graphql new file mode 100644 index 0000000000000..51f8e0a1c356e --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.graphql @@ -0,0 +1,12 @@ +# expected-to-throw +mutation MyMutation { + some_resolver +} + +# %extensions% +type SomeType { + id: ID +} +extend type Mutation { + some_resolver: SomeType @relay_resolver +} diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.expected new file mode 100644 index 0000000000000..22a16c134559b --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.expected @@ -0,0 +1,17 @@ +==================================== INPUT ==================================== +# expected-to-throw +mutation MyMutation { + setName(name: "Alice") { + name @required(action: THROW) + } +} + +# %extensions% +==================================== ERROR ==================================== +✖︎ Unexpected `@required(action: THROW)` directive in mutation response. The use of `@required(action: THROW)` is not supported in mutations. + + mutation_with_required_field.invalid.graphql:4:10 + 3 │ setName(name: "Alice") { + 4 │ name @required(action: THROW) + │ ^^^^^^^^^ + 5 │ } diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.graphql new file mode 100644 index 0000000000000..a146eef66dba3 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.graphql @@ -0,0 +1,8 @@ +# expected-to-throw +mutation MyMutation { + setName(name: "Alice") { + name @required(action: THROW) + } +} + +# %extensions% diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.expected new file mode 100644 index 0000000000000..995d490a3d127 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.expected @@ -0,0 +1,19 @@ +==================================== INPUT ==================================== +# expected-to-throw +mutation MyMutation { + setName(name: "Alice") { + ... on User { + name @required(action: THROW) + } + } +} + +# %extensions% +==================================== ERROR ==================================== +✖︎ Unexpected `@required(action: THROW)` directive in mutation response. The use of `@required(action: THROW)` is not supported in mutations. + + mutation_with_required_field_in_inline_fragment.invalid.graphql:5:12 + 4 │ ... on User { + 5 │ name @required(action: THROW) + │ ^^^^^^^^^ + 6 │ } diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.graphql new file mode 100644 index 0000000000000..a0d303a74e49a --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.graphql @@ -0,0 +1,10 @@ +# expected-to-throw +mutation MyMutation { + setName(name: "Alice") { + ... on User { + name @required(action: THROW) + } + } +} + +# %extensions% diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.expected new file mode 100644 index 0000000000000..1ecc88886781b --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.expected @@ -0,0 +1,11 @@ +==================================== INPUT ==================================== +mutation MyMutation { + setName(name: "Alice") { + name @required(action: LOG) + also_name: name @required(action: NONE) + } +} + +# %extensions% +==================================== OUTPUT =================================== +OK diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.graphql new file mode 100644 index 0000000000000..0660b09372fb7 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.graphql @@ -0,0 +1,8 @@ +mutation MyMutation { + setName(name: "Alice") { + name @required(action: LOG) + also_name: name @required(action: NONE) + } +} + +# %extensions% diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.expected new file mode 100644 index 0000000000000..eba30a14935fe --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.expected @@ -0,0 +1,18 @@ +==================================== INPUT ==================================== +# expected-to-throw +mutation MyMutation { + some_resolver +} + +# %extensions% +extend type Mutation { + some_resolver: Int @relay_resolver +} +==================================== ERROR ==================================== +✖︎ Unexpected `@RelayResolver` field referenced in mutation response. Relay Resolver fields may not be read as part of a mutation response. + + mutation_with_scalar_resolver.invalid.graphql:3:3 + 2 │ mutation MyMutation { + 3 │ some_resolver + │ ^^^^^^^^^^^^^ + 4 │ } diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.graphql new file mode 100644 index 0000000000000..cb14f4f2bec41 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.graphql @@ -0,0 +1,9 @@ +# expected-to-throw +mutation MyMutation { + some_resolver +} + +# %extensions% +extend type Mutation { + some_resolver: Int @relay_resolver +} diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/query_with_required_field.expected b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/query_with_required_field.expected new file mode 100644 index 0000000000000..98111f199977d --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/query_with_required_field.expected @@ -0,0 +1,10 @@ +==================================== INPUT ==================================== +query MyQuery { + me { + name @required(action: THROW) + } +} + +# %extensions% +==================================== OUTPUT =================================== +OK diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/query_with_required_field.graphql b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/query_with_required_field.graphql new file mode 100644 index 0000000000000..e0a92e1feb35a --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/fixtures/query_with_required_field.graphql @@ -0,0 +1,7 @@ +query MyQuery { + me { + name @required(action: THROW) + } +} + +# %extensions% diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/mod.rs b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/mod.rs new file mode 100644 index 0000000000000..c53344c0b8bc5 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations/mod.rs @@ -0,0 +1,44 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +use std::sync::Arc; + +use common::FeatureFlag; +use common::SourceLocationKey; +use fixture_tests::Fixture; +use graphql_ir::build; +use graphql_ir::Program; +use graphql_syntax::parse_executable; +use graphql_test_helpers::diagnostics_to_sorted_string; +use relay_test_schema::get_test_schema_with_extensions; +use relay_transforms::disallow_readtime_features_in_mutations; + +pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result { + let parts: Vec<_> = fixture.content.split("%extensions%").collect(); + + if let [base, extensions] = parts.as_slice() { + let source_location = SourceLocationKey::standalone(fixture.file_name); + let ast = parse_executable(base, source_location) + .map_err(|diagnostics| diagnostics_to_sorted_string(fixture.content, &diagnostics))?; + let schema = get_test_schema_with_extensions(extensions); + + let ir = build(&schema, &ast.definitions) + .map_err(|diagnostics| diagnostics_to_sorted_string(fixture.content, &diagnostics))?; + let program = Program::from_definitions(Arc::clone(&schema), ir); + disallow_readtime_features_in_mutations( + &program, + &FeatureFlag::Disabled, + &FeatureFlag::Disabled, + false, + ) + .map_err(|diagnostics| diagnostics_to_sorted_string(fixture.content, &diagnostics))?; + + Ok("OK".to_owned()) + } else { + panic!("Expected exactly one %extensions% section marker.") + } +} diff --git a/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations_test.rs b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations_test.rs new file mode 100644 index 0000000000000..d499ad40a3e0a --- /dev/null +++ b/compiler/crates/relay-transforms/tests/disallow_readtime_features_in_mutations_test.rs @@ -0,0 +1,62 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @generated SignedSource<> + */ + +mod disallow_readtime_features_in_mutations; + +use disallow_readtime_features_in_mutations::transform_fixture; +use fixture_tests::test_fixture; + +#[tokio::test] +async fn fragment_with_required_spread_in_fragment() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.expected"); + test_fixture(transform_fixture, file!(), "fragment_with_required_spread_in_fragment.graphql", "disallow_readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.expected", input, expected).await; +} + +#[tokio::test] +async fn mutation_with_linked_resolver_invalid() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.expected"); + test_fixture(transform_fixture, file!(), "mutation_with_linked_resolver.invalid.graphql", "disallow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.expected", input, expected).await; +} + +#[tokio::test] +async fn mutation_with_required_field_in_inline_fragment_invalid() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.expected"); + test_fixture(transform_fixture, file!(), "mutation_with_required_field_in_inline_fragment.invalid.graphql", "disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field_in_inline_fragment.invalid.expected", input, expected).await; +} + +#[tokio::test] +async fn mutation_with_required_field_invalid() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.expected"); + test_fixture(transform_fixture, file!(), "mutation_with_required_field.invalid.graphql", "disallow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.expected", input, expected).await; +} + +#[tokio::test] +async fn mutation_with_required_log_or_none_field() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.expected"); + test_fixture(transform_fixture, file!(), "mutation_with_required_log_or_none_field.graphql", "disallow_readtime_features_in_mutations/fixtures/mutation_with_required_log_or_none_field.expected", input, expected).await; +} + +#[tokio::test] +async fn mutation_with_scalar_resolver_invalid() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.expected"); + test_fixture(transform_fixture, file!(), "mutation_with_scalar_resolver.invalid.graphql", "disallow_readtime_features_in_mutations/fixtures/mutation_with_scalar_resolver.invalid.expected", input, expected).await; +} + +#[tokio::test] +async fn query_with_required_field() { + let input = include_str!("disallow_readtime_features_in_mutations/fixtures/query_with_required_field.graphql"); + let expected = include_str!("disallow_readtime_features_in_mutations/fixtures/query_with_required_field.expected"); + test_fixture(transform_fixture, file!(), "query_with_required_field.graphql", "disallow_readtime_features_in_mutations/fixtures/query_with_required_field.expected", input, expected).await; +} diff --git a/compiler/fixture_dirs.txt b/compiler/fixture_dirs.txt index f5e16d7c50fcc..f41350186aa59 100644 --- a/compiler/fixture_dirs.txt +++ b/compiler/fixture_dirs.txt @@ -45,6 +45,7 @@ crates/relay-transforms/tests/client_extensions_abstract_types crates/relay-transforms/tests/declarative_connection crates/relay-transforms/tests/defer_stream crates/relay-transforms/tests/disallow_non_node_id_fields +crates/relay-transforms/tests/disallow_readtime_features_in_mutations crates/relay-transforms/tests/disallow_reserved_aliases crates/relay-transforms/tests/disallow_typename_on_root crates/relay-transforms/tests/flatten diff --git a/packages/relay-runtime/mutations/__tests__/__generated__/commitMutationTestRequiredRootFieldMutation.graphql.js b/packages/relay-runtime/mutations/__tests__/__generated__/commitMutationTestRequiredRootFieldMutation.graphql.js index 9eff776e6fedf..92130bbb739cd 100644 --- a/packages/relay-runtime/mutations/__tests__/__generated__/commitMutationTestRequiredRootFieldMutation.graphql.js +++ b/packages/relay-runtime/mutations/__tests__/__generated__/commitMutationTestRequiredRootFieldMutation.graphql.js @@ -6,7 +6,7 @@ * * @oncall relay * - * @generated SignedSource<> + * @generated SignedSource<<017ca18e8f83a3e70285277746fbb0ba>> * @flow * @lightSyntaxTransform * @nogrep @@ -25,7 +25,7 @@ export type commitMutationTestRequiredRootFieldMutation$variables = {| input?: ?CommentDeleteInput, |}; export type commitMutationTestRequiredRootFieldMutation$data = {| - +commentDelete: {| + +commentDelete: ?{| +deletedCommentId: ?string, |}, |}; @@ -43,44 +43,39 @@ var v0 = [ "name": "input" } ], -v1 = { - "alias": null, - "args": [ - { - "kind": "Variable", - "name": "input", - "variableName": "input" - } - ], - "concreteType": "CommentDeleteResponsePayload", - "kind": "LinkedField", - "name": "commentDelete", - "plural": false, - "selections": [ - { - "alias": null, - "args": null, - "kind": "ScalarField", - "name": "deletedCommentId", - "storageKey": null - } - ], - "storageKey": null -}; +v1 = [ + { + "alias": null, + "args": [ + { + "kind": "Variable", + "name": "input", + "variableName": "input" + } + ], + "concreteType": "CommentDeleteResponsePayload", + "kind": "LinkedField", + "name": "commentDelete", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "deletedCommentId", + "storageKey": null + } + ], + "storageKey": null + } +]; return { "fragment": { "argumentDefinitions": (v0/*: any*/), "kind": "Fragment", "metadata": null, "name": "commitMutationTestRequiredRootFieldMutation", - "selections": [ - { - "kind": "RequiredField", - "field": (v1/*: any*/), - "action": "THROW", - "path": "commentDelete" - } - ], + "selections": (v1/*: any*/), "type": "Mutation", "abstractKey": null }, @@ -89,9 +84,7 @@ return { "argumentDefinitions": (v0/*: any*/), "kind": "Operation", "name": "commitMutationTestRequiredRootFieldMutation", - "selections": [ - (v1/*: any*/) - ] + "selections": (v1/*: any*/) }, "params": { "cacheID": "19f1e1c50328f89205857394403b5d9b", @@ -105,7 +98,7 @@ return { })(); if (__DEV__) { - (node/*: any*/).hash = "c0ee6bcae636236c0564c8da132daeac"; + (node/*: any*/).hash = "b75215ee7b976cd4f043bc5a88b05931"; } module.exports = ((node/*: any*/)/*: Mutation< diff --git a/packages/relay-runtime/mutations/__tests__/commitMutation-test.js b/packages/relay-runtime/mutations/__tests__/commitMutation-test.js index 3d60c2be3f2ae..4cecfe2ea0cce 100644 --- a/packages/relay-runtime/mutations/__tests__/commitMutation-test.js +++ b/packages/relay-runtime/mutations/__tests__/commitMutation-test.js @@ -1169,7 +1169,7 @@ describe('Required mutation roots', () => { mutation commitMutationTestRequiredRootFieldMutation( $input: CommentDeleteInput ) { - commentDelete(input: $input) @required(action: THROW) { + commentDelete(input: $input) { deletedCommentId } }