From 9d4fec5fcad4421adb25f9137e9317cb2c0100ca Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Tue, 23 Apr 2024 20:16:00 -0700 Subject: [PATCH] Report require_custom_scalar_types as a diagnostic instead of panicing --- compiler/Cargo.lock | 1 + .../relay-compiler-playground/src/lib.rs | 5 ++++ .../relay-compiler/src/build_project.rs | 13 +++------- .../src/build_project/build_schema.rs | 7 +++-- .../crates/relay-compiler/src/compiler.rs | 4 +-- .../relay-compiler/src/compiler_state.rs | 11 ++++---- ...lar_not_defined_in_config.invalid.expected | 26 +++++++++++++++++++ ...scalar_not_defined_in_config.invalid.input | 18 +++++++++++++ .../fixtures/project_with_no_js.expected | 10 +++++++ .../fixtures/project_with_no_js.input | 8 ++++++ .../tests/relay_compiler_integration_test.rs | 16 +++++++++++- .../src/server/lsp_state_resources.rs | 8 ++---- compiler/crates/relay-schema/Cargo.toml | 1 + compiler/crates/relay-schema/src/lib.rs | 25 ++++++++++++++++++ compiler/crates/relay-test-schema/src/lib.rs | 8 ++++-- compiler/crates/relay-typegen/src/visit.rs | 2 ++ compiler/crates/schema/src/definitions.rs | 16 ++++++++++-- 17 files changed, 149 insertions(+), 30 deletions(-) create mode 100644 compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.expected create mode 100644 compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.input create mode 100644 compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.expected create mode 100644 compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.input diff --git a/compiler/Cargo.lock b/compiler/Cargo.lock index bed5c1e91c2ed..9ab84f9cf1a19 100644 --- a/compiler/Cargo.lock +++ b/compiler/Cargo.lock @@ -1812,6 +1812,7 @@ dependencies = [ "docblock-shared", "intern", "lazy_static", + "relay-config", "schema", ] diff --git a/compiler/crates/relay-compiler-playground/src/lib.rs b/compiler/crates/relay-compiler-playground/src/lib.rs index 8512231ebd66d..d0402fc826351 100644 --- a/compiler/crates/relay-compiler-playground/src/lib.rs +++ b/compiler/crates/relay-compiler-playground/src/lib.rs @@ -96,6 +96,7 @@ pub fn parse_to_ir_impl(schema_text: &str, document_text: &str) -> PlaygroundRes build_schema_with_extensions( &[(schema_text, SourceLocationKey::generated())], &Vec::<(&str, SourceLocationKey)>::new(), + &Default::default(), ) .map_err(|diagnostics| map_diagnostics(diagnostics, &InputType::Schema(schema_text)))?, ); @@ -132,6 +133,7 @@ pub fn parse_to_reader_ast_impl( build_schema_with_extensions( &[(schema_text, SourceLocationKey::Generated)], &Vec::<(&str, SourceLocationKey)>::new(), + &Default::default(), ) .map_err(|diagnostics| map_diagnostics(diagnostics, &InputType::Schema(schema_text)))?, ); @@ -179,6 +181,7 @@ pub fn parse_to_normalization_ast_impl( build_schema_with_extensions( &[(schema_text, SourceLocationKey::Generated)], &Vec::<(&str, SourceLocationKey)>::new(), + &Default::default(), ) .map_err(|diagnostics| map_diagnostics(diagnostics, &InputType::Schema(schema_text)))?, ); @@ -225,6 +228,7 @@ pub fn parse_to_types_impl( build_schema_with_extensions( &[(schema_text, SourceLocationKey::Generated)], &Vec::<(&str, SourceLocationKey)>::new(), + &Default::default(), ) .map_err(|diagnostics| map_diagnostics(diagnostics, &InputType::Schema(schema_text)))?, ); @@ -282,6 +286,7 @@ fn transform_impl( build_schema_with_extensions( &[(schema_text, SourceLocationKey::Generated)], &Vec::<(&str, SourceLocationKey)>::new(), + &Default::default(), ) .map_err(|diagnostics| map_diagnostics(diagnostics, &InputType::Schema(schema_text)))?, ); diff --git a/compiler/crates/relay-compiler/src/build_project.rs b/compiler/crates/relay-compiler/src/build_project.rs index ca7310953e1e7..1f650726ae156 100644 --- a/compiler/crates/relay-compiler/src/build_project.rs +++ b/compiler/crates/relay-compiler/src/build_project.rs @@ -208,20 +208,13 @@ pub fn build_programs( let mut build_mode = if !compiler_state.has_processed_changes() { BuildMode::Full } else { - let project_schema_change = compiler_state.schema_change_safety( - log_event, - project_name, - &project_config.schema_config, - ); + let project_schema_change = + compiler_state.schema_change_safety(log_event, project_name, &project_config); match project_schema_change { SchemaChangeSafety::Unsafe => BuildMode::Full, SchemaChangeSafety::Safe | SchemaChangeSafety::SafeWithIncrementalBuild(_) => { let base_schema_change = if let Some(base) = project_config.base { - compiler_state.schema_change_safety( - log_event, - base, - &project_config.schema_config, - ) + compiler_state.schema_change_safety(log_event, base, &project_config) } else { SchemaChangeSafety::Safe }; diff --git a/compiler/crates/relay-compiler/src/build_project/build_schema.rs b/compiler/crates/relay-compiler/src/build_project/build_schema.rs index 07b66a2e11f8c..38b98816b6a8b 100644 --- a/compiler/crates/relay-compiler/src/build_project/build_schema.rs +++ b/compiler/crates/relay-compiler/src/build_project/build_schema.rs @@ -48,8 +48,11 @@ pub fn build_schema( .into_iter() .map(|(schema, location_key)| (schema.as_str(), location_key)), ); - let mut schema = - relay_schema::build_schema_with_extensions(&schema_sources, &extensions)?; + let mut schema = relay_schema::build_schema_with_extensions( + &schema_sources, + &extensions, + project_config, + )?; if project_config.feature_flags.enable_relay_resolver_transform { extend_schema_with_resolvers( diff --git a/compiler/crates/relay-compiler/src/compiler.rs b/compiler/crates/relay-compiler/src/compiler.rs index 8e6d1951138c1..66a027ace6878 100644 --- a/compiler/crates/relay-compiler/src/compiler.rs +++ b/compiler/crates/relay-compiler/src/compiler.rs @@ -370,8 +370,8 @@ async fn build_projects( .unwrap_or_else(|| Arc::new(ArtifactMapKind::Unconnected(Default::default()))); let mut removed_artifact_sources = graphql_asts .remove(&project_name) - .expect("Expect GraphQLAsts to exist.") - .removed_definition_names; + .map(|ast| ast.removed_definition_names) + .unwrap_or_else(|| Vec::new()); // We may have no js files in this project yet let removed_docblock_artifact_sources = get_removed_docblock_artifact_source_keys(compiler_state.docblocks.get(&project_name)); diff --git a/compiler/crates/relay-compiler/src/compiler_state.rs b/compiler/crates/relay-compiler/src/compiler_state.rs index cd0fe5f38e41f..c46a094bed4bf 100644 --- a/compiler/crates/relay-compiler/src/compiler_state.rs +++ b/compiler/crates/relay-compiler/src/compiler_state.rs @@ -28,8 +28,8 @@ use fnv::FnvBuildHasher; use fnv::FnvHashMap; use fnv::FnvHashSet; use rayon::prelude::*; +use relay_config::ProjectConfig; use relay_config::ProjectName; -use relay_config::SchemaConfig; use schema::SDLSchema; use schema_diff::check::SchemaChangeSafety; use schema_diff::definitions::SchemaChange; @@ -451,7 +451,7 @@ impl CompilerState { &self, sources: &SchemaSources, schema_change: SchemaChange, - schema_config: &SchemaConfig, + project_config: &ProjectConfig, ) -> SchemaChangeSafety { if schema_change == SchemaChange::None { SchemaChangeSafety::Safe @@ -465,8 +465,9 @@ impl CompilerState { match relay_schema::build_schema_with_extensions( ¤t_sources_with_location, &Vec::<(&str, SourceLocationKey)>::new(), + &project_config, ) { - Ok(schema) => schema_change.get_safety(&schema, schema_config), + Ok(schema) => schema_change.get_safety(&schema, &project_config.schema_config), Err(_) => SchemaChangeSafety::Unsafe, } } @@ -477,7 +478,7 @@ impl CompilerState { &self, log_event: &impl PerfLogEvent, project_name: ProjectName, - schema_config: &SchemaConfig, + project_config: &ProjectConfig, ) -> SchemaChangeSafety { if let Some(extension) = self.extensions.get(&project_name) { if !extension.pending.is_empty() { @@ -502,7 +503,7 @@ impl CompilerState { let schema_change = self.get_schema_change(schema); let schema_change_string = schema_change.to_string(); let schema_change_safety = - self.get_schema_change_safety(schema, schema_change, schema_config); + self.get_schema_change_safety(schema, schema_change, project_config); match schema_change_safety { SchemaChangeSafety::Unsafe => { log_event.string("schema_change", schema_change_string); diff --git a/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.expected b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.expected new file mode 100644 index 0000000000000..e49b56cd16890 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.expected @@ -0,0 +1,26 @@ +==================================== INPUT ==================================== +==================================== INPUT ==================================== +//- foo.js +graphql` + fragment foo on Query { + greeting + }`; + +//- relay.config.json +{ + "language": "typescript", + "schema": "./schema.graphql", + "requireCustomScalarTypes": true +} + +//- schema.graphql +type Query { greeting: MyScalar } + +scalar MyScalar # Not present in config +==================================== OUTPUT =================================== +✖︎ Expected the JS type for 'MyScalar' to be defined, please update 'customScalarTypes' in your compiler config. + + schema.graphql:3:8 + 2 │ + 3 │ scalar MyScalar # Not present in config + │ ^^^^^^^^ diff --git a/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.input b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.input new file mode 100644 index 0000000000000..1b8543981d6e4 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.input @@ -0,0 +1,18 @@ +==================================== INPUT ==================================== +//- foo.js +graphql` + fragment foo on Query { + greeting + }`; + +//- relay.config.json +{ + "language": "typescript", + "schema": "./schema.graphql", + "requireCustomScalarTypes": true +} + +//- schema.graphql +type Query { greeting: MyScalar } + +scalar MyScalar # Not present in config \ No newline at end of file diff --git a/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.expected b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.expected new file mode 100644 index 0000000000000..7f9e76db3fab4 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.expected @@ -0,0 +1,10 @@ +==================================== INPUT ==================================== +//- relay.config.json +{ + "language": "typescript", + "schema": "./schema.graphql" +} + +//- schema.graphql +type Query { greeting: String } +==================================== OUTPUT =================================== diff --git a/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.input b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.input new file mode 100644 index 0000000000000..2af5558ff9167 --- /dev/null +++ b/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/project_with_no_js.input @@ -0,0 +1,8 @@ +//- relay.config.json +{ + "language": "typescript", + "schema": "./schema.graphql" +} + +//- schema.graphql +type Query { greeting: String } diff --git a/compiler/crates/relay-compiler/tests/relay_compiler_integration_test.rs b/compiler/crates/relay-compiler/tests/relay_compiler_integration_test.rs index 7eaafa82319ed..5e32e6186ee27 100644 --- a/compiler/crates/relay-compiler/tests/relay_compiler_integration_test.rs +++ b/compiler/crates/relay-compiler/tests/relay_compiler_integration_test.rs @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<> */ mod relay_compiler_integration; @@ -47,6 +47,13 @@ async fn client_mutation_resolver_invalid_nonscalar() { test_fixture(transform_fixture, file!(), "client_mutation_resolver_invalid_nonscalar.input", "relay_compiler_integration/fixtures/client_mutation_resolver_invalid_nonscalar.expected", input, expected).await; } +#[tokio::test] +async fn custom_scalar_not_defined_in_config_invalid() { + let input = include_str!("relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.input"); + let expected = include_str!("relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.expected"); + test_fixture(transform_fixture, file!(), "custom_scalar_not_defined_in_config.invalid.input", "relay_compiler_integration/fixtures/custom_scalar_not_defined_in_config.invalid.expected", input, expected).await; +} + #[tokio::test] async fn custom_scalar_variable_default_arg_invalid() { let input = include_str!("relay_compiler_integration/fixtures/custom_scalar_variable_default_arg.invalid.input"); @@ -96,6 +103,13 @@ async fn preloadable_query_typescript() { test_fixture(transform_fixture, file!(), "preloadable_query_typescript.input", "relay_compiler_integration/fixtures/preloadable_query_typescript.expected", input, expected).await; } +#[tokio::test] +async fn project_with_no_js() { + let input = include_str!("relay_compiler_integration/fixtures/project_with_no_js.input"); + let expected = include_str!("relay_compiler_integration/fixtures/project_with_no_js.expected"); + test_fixture(transform_fixture, file!(), "project_with_no_js.input", "relay_compiler_integration/fixtures/project_with_no_js.expected", input, expected).await; +} + #[tokio::test] async fn resolver_on_interface() { let input = include_str!("relay_compiler_integration/fixtures/resolver_on_interface.input"); diff --git a/compiler/crates/relay-lsp/src/server/lsp_state_resources.rs b/compiler/crates/relay-lsp/src/server/lsp_state_resources.rs index 21ccf9563796e..da461088e2e89 100644 --- a/compiler/crates/relay-lsp/src/server/lsp_state_resources.rs +++ b/compiler/crates/relay-lsp/src/server/lsp_state_resources.rs @@ -408,17 +408,13 @@ impl BuildMode::Full, SchemaChangeSafety::Safe | SchemaChangeSafety::SafeWithIncrementalBuild(_) => { let base_schema_change = if let Some(base) = project_config.base { - compiler_state.schema_change_safety( - log_event, - base, - &project_config.schema_config, - ) + compiler_state.schema_change_safety(log_event, base, &project_config) } else { SchemaChangeSafety::Safe }; diff --git a/compiler/crates/relay-schema/Cargo.toml b/compiler/crates/relay-schema/Cargo.toml index eeda391d3ca2e..d7082008db86d 100644 --- a/compiler/crates/relay-schema/Cargo.toml +++ b/compiler/crates/relay-schema/Cargo.toml @@ -11,6 +11,7 @@ license = "MIT" [dependencies] common = { path = "../common" } docblock-shared = { path = "../docblock-shared" } +relay-config = { path = "../relay-config" } intern = { path = "../intern" } lazy_static = "1.4" schema = { path = "../schema" } diff --git a/compiler/crates/relay-schema/src/lib.rs b/compiler/crates/relay-schema/src/lib.rs index f9bd8af8d9473..5952ac86741ae 100644 --- a/compiler/crates/relay-schema/src/lib.rs +++ b/compiler/crates/relay-schema/src/lib.rs @@ -16,13 +16,16 @@ use std::iter::once; use ::intern::string_key::StringKey; use common::ArgumentName; +use common::Diagnostic; use common::DiagnosticsResult; use common::DirectiveName; use common::SourceLocationKey; use intern::intern; use lazy_static::lazy_static; +use relay_config::ProjectConfig; use schema::ArgumentDefinitions; use schema::SDLSchema; +use schema::Schema; use schema::TypeReference; const RELAY_EXTENSIONS: &str = include_str!("./relay-extensions.graphql"); @@ -42,6 +45,7 @@ pub fn build_schema_with_extensions< >( server_sdls: &[(T, SourceLocationKey)], extension_sdls: &[(U, SourceLocationKey)], + project_config: &ProjectConfig, ) -> DiagnosticsResult { let extensions: Vec<(&str, SourceLocationKey)> = once((RELAY_EXTENSIONS, SourceLocationKey::generated())) @@ -54,6 +58,27 @@ pub fn build_schema_with_extensions< let mut schema = schema::build_schema_with_extensions(server_sdls, &extensions)?; + if project_config.typegen_config.require_custom_scalar_types { + for scalar in schema.scalars() { + if scalar.is_builtin_type() { + continue; + } + if !project_config + .typegen_config + .custom_scalar_types + .contains_key(&scalar.name.item) + { + return Err(vec![Diagnostic::error( + format!( + "Expected the JS type for '{}' to be defined, please update 'customScalarTypes' in your compiler config.", + scalar.name.item + ), + scalar.name.location, + )]); + } + } + } + // Remove label arg from @defer and @stream directives since the compiler // adds these arguments. for directive_name in &[*DEFER, *STREAM] { diff --git a/compiler/crates/relay-test-schema/src/lib.rs b/compiler/crates/relay-test-schema/src/lib.rs index d8807770e7149..85ef84a21ecae 100644 --- a/compiler/crates/relay-test-schema/src/lib.rs +++ b/compiler/crates/relay-test-schema/src/lib.rs @@ -23,7 +23,8 @@ lazy_static! { pub static ref TEST_SCHEMA: Arc = Arc::new( build_schema_with_extensions::<_, &str>( &[(TEST_SCHEMA_DATA, SourceLocationKey::generated())], - &[] + &[], + &Default::default(), ) .expect("Expected test schema to be valid") ); @@ -33,7 +34,8 @@ lazy_static! { TEST_SCHEMA_WITH_CUSTOM_ID_DATA, SourceLocationKey::generated() )], - &[] + &[], + &Default::default(), ) .expect("Expected test schema to be valid") ); @@ -55,6 +57,7 @@ pub fn get_test_schema_with_located_extensions( build_schema_with_extensions( &[(TEST_SCHEMA_DATA, SourceLocationKey::generated())], &[(extensions_sdl, source_location)], + &Default::default(), ) .expect("Expected test schema (and extensions) to be valid"), ) @@ -72,6 +75,7 @@ pub fn get_test_schema_with_custom_id_with_extensions(extensions_sdl: &str) -> A SourceLocationKey::generated(), )], &[(extensions_sdl, SourceLocationKey::generated())], + &Default::default(), ) .expect("Expected test schema (and extensions) to be valid"), ) diff --git a/compiler/crates/relay-typegen/src/visit.rs b/compiler/crates/relay-typegen/src/visit.rs index 10662ec3f6f65..c3c1b4866d8e9 100644 --- a/compiler/crates/relay-typegen/src/visit.rs +++ b/compiler/crates/relay-typegen/src/visit.rs @@ -1960,6 +1960,8 @@ fn transform_graphql_scalar_type( .typegen_config .require_custom_scalar_types { + // There is a diagnostic in relay-schema which checks this when the schema is built. We should never hit this + // condition in practice. panic!( "Expected the JS type for '{}' to be defined, please update 'customScalarTypes' in your compiler config.", scalar_name.item diff --git a/compiler/crates/schema/src/definitions.rs b/compiler/crates/schema/src/definitions.rs index 2c7d063402b8f..cbead08e2975e 100644 --- a/compiler/crates/schema/src/definitions.rs +++ b/compiler/crates/schema/src/definitions.rs @@ -12,6 +12,9 @@ use std::fmt; use std::hash::Hash; use std::slice::Iter; +use ::intern::intern; +use ::intern::string_key::Intern; +use ::intern::string_key::StringKey; use common::ArgumentName; use common::DirectiveName; use common::EnumName; @@ -26,8 +29,6 @@ use common::WithLocation; use graphql_syntax::ConstantValue; use graphql_syntax::DirectiveLocation; pub use interface::*; -use intern::string_key::Intern; -use intern::string_key::StringKey; use lazy_static::lazy_static; use crate::Schema; @@ -386,6 +387,17 @@ pub struct Scalar { pub hack_source: Option, } +impl Scalar { + pub fn is_builtin_type(&self) -> bool { + let name = self.name.item.0; + name == intern!("String") + || name == intern!("Int") + || name == intern!("Float") + || name == intern!("Boolean") + || name == intern!("ID") + } +} + #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub struct Object { pub name: WithLocation,