Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report require_custom_scalar_types as a diagnostic instead of panicking #4682

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions compiler/crates/relay-compiler-playground/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))?,
);
Expand Down Expand Up @@ -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)))?,
);
Expand Down Expand Up @@ -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)))?,
);
Expand Down Expand Up @@ -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)))?,
);
Expand Down Expand Up @@ -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)))?,
);
Expand Down
13 changes: 3 additions & 10 deletions compiler/crates/relay-compiler/src/build_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions compiler/crates/relay-compiler/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ async fn build_projects<TPerfLogger: PerfLogger + 'static>(
.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));
Expand Down
11 changes: 6 additions & 5 deletions compiler/crates/relay-compiler/src/compiler_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -465,8 +465,9 @@ impl CompilerState {
match relay_schema::build_schema_with_extensions(
&current_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,
}
}
Expand All @@ -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() {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
│ ^^^^^^^^
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
==================================== INPUT ====================================
//- relay.config.json
{
"language": "typescript",
"schema": "./schema.graphql"
}

//- schema.graphql
type Query { greeting: String }
==================================== OUTPUT ===================================
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//- relay.config.json
{
"language": "typescript",
"schema": "./schema.graphql"
}

//- schema.graphql
type Query { greeting: String }
Original file line number Diff line number Diff line change
Expand Up @@ -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<<f79ec1771e29102c917c2acdb8bdb98d>>
* @generated SignedSource<<adeb8226ad6a02ceb0968abce186b942>>
*/

mod relay_compiler_integration;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
8 changes: 2 additions & 6 deletions compiler/crates/relay-lsp/src/server/lsp_state_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,17 +408,13 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio
let project_schema_change = compiler_state.schema_change_safety(
log_event,
project_config.name,
&project_config.schema_config,
&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
};
Expand Down
1 change: 1 addition & 0 deletions compiler/crates/relay-schema/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
25 changes: 25 additions & 0 deletions compiler/crates/relay-schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -42,6 +45,7 @@ pub fn build_schema_with_extensions<
>(
server_sdls: &[(T, SourceLocationKey)],
extension_sdls: &[(U, SourceLocationKey)],
project_config: &ProjectConfig,
) -> DiagnosticsResult<SDLSchema> {
let extensions: Vec<(&str, SourceLocationKey)> =
once((RELAY_EXTENSIONS, SourceLocationKey::generated()))
Expand All @@ -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] {
Expand Down
8 changes: 6 additions & 2 deletions compiler/crates/relay-test-schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ lazy_static! {
pub static ref TEST_SCHEMA: Arc<SDLSchema> = Arc::new(
build_schema_with_extensions::<_, &str>(
&[(TEST_SCHEMA_DATA, SourceLocationKey::generated())],
&[]
&[],
&Default::default(),
)
.expect("Expected test schema to be valid")
);
Expand All @@ -33,7 +34,8 @@ lazy_static! {
TEST_SCHEMA_WITH_CUSTOM_ID_DATA,
SourceLocationKey::generated()
)],
&[]
&[],
&Default::default(),
)
.expect("Expected test schema to be valid")
);
Expand All @@ -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"),
)
Expand All @@ -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"),
)
Expand Down
2 changes: 2 additions & 0 deletions compiler/crates/relay-typegen/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading