Skip to content

Commit febe8d3

Browse files
dariuszkucdariuszkuc
authored
feat(compiler): allow SDL to contain built-in type redefinition (#994)
Updates `SchemaBuilder` to allow SDL to contain built-in meta types (e.g. `__Type`) redefinitions. GraphQL spec doesn't really define the expected behavior for handling SDL with redeclared meta fields (see: [#1036](graphql/graphql-spec#1036), [#1049](graphql/graphql-spec#1049)). `graphql-js` [reference implementation](https://github.com/graphql/graphql-js/blob/v16.11.0/src/utilities/extendSchema.ts#L187) simply ignores those redefinitions even if the underlying type definitions don't match. This PR updates `ignore_builtin_redefinitions` option to also ignore meta types redefinitions (previously it was only allowing built-in scalar redefinitions #990).
1 parent 1e79063 commit febe8d3

File tree

5 files changed

+277
-47
lines changed

5 files changed

+277
-47
lines changed

‎crates/apollo-compiler/CHANGELOG.md‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,20 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2121

2222
## Features
2323

24-
- **Adds `ignore_builtin_redefinitions` option to `SchemaBuilder` to allow SDL to contain built-in
25-
scalar definitions - [dariuszkuc], [pull/990]**
24+
- **Adds `ignore_builtin_redefinitions` option to `SchemaBuilder` to allow SDL to contain built-in type
25+
definitions - [dariuszkuc], [pull/990] and [pull/994]**
2626

2727
## Fixes
2828

29-
- **Fix handling of orphan root type extensions - [dariuszkuc], [pull/993]**
30-
- **Fix possible stack overflow in validation of directive definition arguments with nested types - [dariuszkuc], [pull/987]**
31-
- **Fix `iter_origin()` to be a pub method- [duckki], [pull/989]**
29+
- **Fix handling of orphan root type extensions - [dariuszkuc], [pull/993](#993)**
30+
- **Fix possible stack overflow in validation of directive definition arguments with nested types - [dariuszkuc], [pull/987](#987)**
31+
- **Fix `iter_origin()` to be a pub method- [duckki], [pull/989](#989)**
32+
33+
[pull/994]: https://github.com/apollographql/apollo-rs/pull/994
34+
[pull/993]: https://github.com/apollographql/apollo-rs/pull/993
35+
[pull/990]: https://github.com/apollographql/apollo-rs/pull/990
36+
[pull/989]: https://github.com/apollographql/apollo-rs/pull/989
37+
[pull/987]: https://github.com/apollographql/apollo-rs/pull/987
3238

3339
# [1.29.0](https://crates.io/crates/apollo-compiler/1.29.0) - 2025-08-08
3440

‎crates/apollo-compiler/src/schema/from_ast.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl SchemaBuilder {
7878
self
7979
}
8080

81-
/// Configure the builder to allow SDL to contain scalar built-in types re-definitions.
81+
/// Configure the builder to allow SDL to contain built-in type re-definitions.
8282
/// Re-definitions are going to be effectively ignored and compiler will continue to use
8383
/// built-in GraphQL spec definitions.
8484
pub fn ignore_builtin_redefinitions(mut self) -> Self {
@@ -134,15 +134,15 @@ impl SchemaBuilder {
134134
}
135135
Entry::Occupied(entry) => {
136136
let previous = entry.get();
137+
if self.ignore_builtin_redefinitions && previous.is_built_in() {
138+
continue;
139+
}
140+
137141
if $is_scalar && previous.is_built_in() {
138-
if self.ignore_builtin_redefinitions {
139-
continue;
140-
} else {
141-
self.errors.push(
142-
$def.location(),
143-
BuildError::BuiltInScalarTypeRedefinition,
144-
)
145-
}
142+
self.errors.push(
143+
$def.location(),
144+
BuildError::BuiltInScalarTypeRedefinition,
145+
)
146146
} else {
147147
self.errors.push(
148148
$def.name.location(),

‎crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
use apollo_compiler::schema::SchemaBuilder;
2+
use apollo_compiler::Schema;
3+
use expect_test::expect;
4+
5+
#[test]
6+
fn handles_built_in_scalar_redefinition() {
7+
let schema = r#"
8+
scalar String
9+
scalar Int
10+
scalar Float
11+
scalar Boolean
12+
scalar ID
13+
14+
type Query {
15+
foo: String
16+
}
17+
"#;
18+
19+
let errors = Schema::parse_and_validate(schema, "schema.graphql")
20+
.expect_err("valid schema")
21+
.errors;
22+
let expected = expect![[r#"
23+
Error: built-in scalar definitions must be omitted
24+
╭─[ schema.graphql:2:3 ]
25+
│
26+
2 │ scalar String
27+
│ ──────┬──────
28+
│ ╰──────── remove this scalar definition
29+
───╯
30+
Error: built-in scalar definitions must be omitted
31+
╭─[ schema.graphql:3:3 ]
32+
│
33+
3 │ scalar Int
34+
│ ─────┬────
35+
│ ╰────── remove this scalar definition
36+
───╯
37+
Error: built-in scalar definitions must be omitted
38+
╭─[ schema.graphql:4:3 ]
39+
│
40+
4 │ scalar Float
41+
│ ──────┬─────
42+
│ ╰─────── remove this scalar definition
43+
───╯
44+
Error: built-in scalar definitions must be omitted
45+
╭─[ schema.graphql:5:3 ]
46+
│
47+
5 │ scalar Boolean
48+
│ ───────┬──────
49+
│ ╰──────── remove this scalar definition
50+
───╯
51+
Error: built-in scalar definitions must be omitted
52+
╭─[ schema.graphql:6:3 ]
53+
│
54+
6 │ scalar ID
55+
│ ────┬────
56+
│ ╰────── remove this scalar definition
57+
───╯
58+
"#]];
59+
expected.assert_eq(&errors.to_string());
60+
61+
let builder = SchemaBuilder::new().ignore_builtin_redefinitions();
62+
let _ = builder
63+
.parse(schema, "schema.graphql")
64+
.build()
65+
.expect("schema parsed successfully");
66+
}
67+
68+
#[test]
69+
fn handles_built_in_type_redefinition() {
70+
let schema = r#"
71+
type __Directive {
72+
name: String!
73+
description: String!
74+
isRepeatable: String!
75+
args: __InputValue
76+
locations: String!
77+
}
78+
79+
type __Schema {
80+
description: String
81+
types: [__Type!]
82+
queryType: __Type!
83+
mutationType: __Type
84+
subscriptionType: __Type
85+
directives: [__Directive!]
86+
}
87+
88+
type __Type {
89+
kind: __TypeKind!
90+
name: String
91+
description: String
92+
fields: [__Field!]
93+
interfaces: [__Type!]
94+
possibleTypes: [__Type!]
95+
enumValues: [__EnumValue!]
96+
inputFields: [__InputValue!]
97+
ofType: __Type
98+
specifiedByURL: String
99+
}
100+
101+
enum __TypeKind {
102+
SCALAR
103+
OBJECT
104+
INTERFACE
105+
UNION
106+
ENUM
107+
INPUT_OBJECT
108+
LIST
109+
}
110+
111+
type __Field {
112+
name: String!
113+
description: String
114+
args: [__InputValue!]!
115+
type: __Type!
116+
isDeprecated: Boolean!
117+
deprecationReason: String
118+
}
119+
120+
type __InputValue {
121+
name: String!
122+
description: String
123+
type: __Type!
124+
defaultValue: String
125+
deprecationReason: String
126+
}
127+
128+
type __EnumValue {
129+
name: String!
130+
description: String
131+
deprecationReason: String
132+
}
133+
134+
type Query {
135+
foo: String
136+
}
137+
"#;
138+
139+
let errors = Schema::parse_and_validate(schema, "schema.graphql")
140+
.expect_err("valid schema")
141+
.errors;
142+
let expected = expect![[r#"
143+
Error: the type `__Directive` is defined multiple times in the schema
144+
╭─[ built_in.graphql:87:6 ]
145+
│
146+
87 │ type __Directive {
147+
│ ─────┬─────
148+
│ ╰─────── previous definition of `__Directive` here
149+
│
150+
├─[ schema.graphql:2:11 ]
151+
│
152+
2 │ type __Directive {
153+
│ ─────┬─────
154+
│ ╰─────── `__Directive` redefined here
155+
│
156+
│ Help: remove or rename one of the definitions, or use `extend`
157+
────╯
158+
Error: the type `__Schema` is defined multiple times in the schema
159+
╭─[ built_in.graphql:2:6 ]
160+
│
161+
2 │ type __Schema {
162+
│ ────┬───
163+
│ ╰───── previous definition of `__Schema` here
164+
│
165+
├─[ schema.graphql:10:11 ]
166+
│
167+
10 │ type __Schema {
168+
│ ────┬───
169+
│ ╰───── `__Schema` redefined here
170+
│
171+
│ Help: remove or rename one of the definitions, or use `extend`
172+
────╯
173+
Error: the type `__Type` is defined multiple times in the schema
174+
╭─[ built_in.graphql:17:6 ]
175+
│
176+
17 │ type __Type {
177+
│ ───┬──
178+
│ ╰──── previous definition of `__Type` here
179+
│
180+
├─[ schema.graphql:19:11 ]
181+
│
182+
19 │ type __Type {
183+
│ ───┬──
184+
│ ╰──── `__Type` redefined here
185+
│
186+
│ Help: remove or rename one of the definitions, or use `extend`
187+
────╯
188+
Error: the type `__TypeKind` is defined multiple times in the schema
189+
╭─[ built_in.graphql:38:6 ]
190+
│
191+
38 │ enum __TypeKind {
192+
│ ─────┬────
193+
│ ╰────── previous definition of `__TypeKind` here
194+
│
195+
├─[ schema.graphql:32:11 ]
196+
│
197+
32 │ enum __TypeKind {
198+
│ ─────┬────
199+
│ ╰────── `__TypeKind` redefined here
200+
│
201+
│ Help: remove or rename one of the definitions, or use `extend`
202+
────╯
203+
Error: the type `__Field` is defined multiple times in the schema
204+
╭─[ built_in.graphql:58:6 ]
205+
│
206+
58 │ type __Field {
207+
│ ───┬───
208+
│ ╰───── previous definition of `__Field` here
209+
│
210+
├─[ schema.graphql:42:11 ]
211+
│
212+
42 │ type __Field {
213+
│ ───┬───
214+
│ ╰───── `__Field` redefined here
215+
│
216+
│ Help: remove or rename one of the definitions, or use `extend`
217+
────╯
218+
Error: the type `__InputValue` is defined multiple times in the schema
219+
╭─[ built_in.graphql:68:6 ]
220+
│
221+
68 │ type __InputValue {
222+
│ ──────┬─────
223+
│ ╰─────── previous definition of `__InputValue` here
224+
│
225+
├─[ schema.graphql:51:11 ]
226+
│
227+
51 │ type __InputValue {
228+
│ ──────┬─────
229+
│ ╰─────── `__InputValue` redefined here
230+
│
231+
│ Help: remove or rename one of the definitions, or use `extend`
232+
────╯
233+
Error: the type `__EnumValue` is defined multiple times in the schema
234+
╭─[ built_in.graphql:79:6 ]
235+
│
236+
79 │ type __EnumValue {
237+
│ ─────┬─────
238+
│ ╰─────── previous definition of `__EnumValue` here
239+
│
240+
├─[ schema.graphql:59:11 ]
241+
│
242+
59 │ type __EnumValue {
243+
│ ─────┬─────
244+
│ ╰─────── `__EnumValue` redefined here
245+
│
246+
│ Help: remove or rename one of the definitions, or use `extend`
247+
────╯
248+
"#]];
249+
expected.assert_eq(&errors.to_string());
250+
251+
let builder = SchemaBuilder::new().ignore_builtin_redefinitions();
252+
let _ = builder
253+
.parse(schema, "schema.graphql")
254+
.build()
255+
.expect("schema parsed successfully");
256+
}

‎crates/apollo-compiler/tests/validation/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
mod field_merging;
2+
mod ignore_builtin_redefinition;
23
mod interface;
34
mod object;
45
mod operation;

‎crates/apollo-compiler/tests/validation/types.rs‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
//! Ported from graphql-js, 2023-11-16
22
//! https://github.com/graphql/graphql-js/blob/0b7590f0a2b65e6210da2e49be0d8e6c27781af2/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts
3-
use apollo_compiler::schema::SchemaBuilder;
43
use apollo_compiler::validation::Valid;
54
use apollo_compiler::ExecutableDocument;
65
use apollo_compiler::Schema;
7-
use expect_test::expect;
86
use expect_test::Expect;
97
use std::sync::OnceLock;
108
use unindent::unindent;
@@ -2018,34 +2016,3 @@ mod variable_default_values {
20182016
);
20192017
}
20202018
}
2021-
2022-
#[test]
2023-
fn handles_built_in_type_redefinition() {
2024-
let schema = r#"
2025-
scalar String
2026-
2027-
type Query {
2028-
foo: String
2029-
}
2030-
"#;
2031-
2032-
let errors = Schema::parse_and_validate(schema, "schema.graphql")
2033-
.expect_err("invalid schema")
2034-
.errors;
2035-
let expected = expect![[r#"
2036-
Error: built-in scalar definitions must be omitted
2037-
╭─[ schema.graphql:2:1 ]
2038-
│
2039-
2 │ scalar String
2040-
│ ──────┬──────
2041-
│ ╰──────── remove this scalar definition
2042-
───╯
2043-
"#]];
2044-
expected.assert_eq(&errors.to_string());
2045-
2046-
let builder = SchemaBuilder::new().ignore_builtin_redefinitions();
2047-
let _ = builder
2048-
.parse(schema, "schema.graphql")
2049-
.build()
2050-
.expect("schema parsed successfully");
2051-
}

0 commit comments

Comments
 (0)