Skip to content

Commit e71ab6c

Browse files
committed
Fix variable validation failing when type loader overrides built-in scalars
When a type loader returns a custom instance for a built-in scalar (e.g. ID, String), `Schema::getType()` stores and returns that instance. However, field argument definitions in the schema still reference the built-in singletons (e.g. `Type::id()`). During query validation, `VariablesInAllowedPosition` resolves variable types via `Schema::getType()` and compares them against argument types using `TypeComparators`. Both `isEqualType()` and `isTypeSubTypeOf()` use strict identity checks (`===`), which fail when comparing two different instances that represent the same named type. This causes queries with variables of overridden built-in scalar types to fail validation with errors like: Variable "$id" of type "ID!" used in position expecting type "ID!". The fix adds a name-based equality check for `NamedType` instances in both `isEqualType()` and `isTypeSubTypeOf()`, falling back to comparing type names when the identity check fails. In a valid schema, each type name is unique, so two named types with the same name are semantically equivalent even if they are different object instances. Ref webonyx#1869 Fixes webonyx#1874
1 parent 74790a3 commit e71ab6c

2 files changed

Lines changed: 82 additions & 0 deletions

File tree

src/Utils/TypeComparators.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use GraphQL\Error\InvariantViolation;
66
use GraphQL\Type\Definition\ImplementingType;
77
use GraphQL\Type\Definition\ListOfType;
8+
use GraphQL\Type\Definition\NamedType;
89
use GraphQL\Type\Definition\NonNull;
910
use GraphQL\Type\Definition\Type;
1011
use GraphQL\Type\Schema;
@@ -19,6 +20,14 @@ public static function isEqualType(Type $typeA, Type $typeB): bool
1920
return true;
2021
}
2122

23+
// Named types with the same name are equal, even if they are different
24+
// instances (e.g. a type loader override vs the built-in singleton).
25+
if ($typeA instanceof NamedType && $typeB instanceof NamedType
26+
&& $typeA->name() === $typeB->name()
27+
) {
28+
return true;
29+
}
30+
2231
// If either type is non-null, the other must also be non-null.
2332
if ($typeA instanceof NonNull && $typeB instanceof NonNull) {
2433
return self::isEqualType($typeA->getWrappedType(), $typeB->getWrappedType());
@@ -46,6 +55,14 @@ public static function isTypeSubTypeOf(Schema $schema, Type $maybeSubType, Type
4655
return true;
4756
}
4857

58+
// Named types with the same name are equivalent, even if they are different
59+
// instances (e.g. a type loader override vs the built-in singleton).
60+
if ($maybeSubType instanceof NamedType && $superType instanceof NamedType
61+
&& $maybeSubType->name() === $superType->name()
62+
) {
63+
return true;
64+
}
65+
4966
// If superType is non-null, maybeSubType must also be nullable.
5067
if ($superType instanceof NonNull) {
5168
if ($maybeSubType instanceof NonNull) {

tests/Type/ScalarOverridesTest.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use GraphQL\Error\InvariantViolation;
66
use GraphQL\GraphQL;
77
use GraphQL\Type\Definition\CustomScalarType;
8+
use GraphQL\Type\Definition\NonNull;
89
use GraphQL\Type\Definition\ObjectType;
910
use GraphQL\Type\Definition\ScalarType;
1011
use GraphQL\Type\Definition\Type;
@@ -204,6 +205,70 @@ public function testNonOverriddenScalarsAreUnaffected(): void
204205
self::assertSame('abc-123', $data['identifier']);
205206
}
206207

208+
public function testTypeLoaderOverrideWithVariableOfOverriddenBuiltInScalarType(): void
209+
{
210+
$customID = new CustomScalarType([
211+
'name' => Type::ID,
212+
'serialize' => static fn ($value): string => (string) $value,
213+
'parseValue' => static fn ($value): string => (string) $value,
214+
]);
215+
216+
$queryType = new ObjectType([
217+
'name' => 'Query',
218+
'fields' => [
219+
'node' => [
220+
'type' => Type::string(),
221+
'args' => [
222+
'id' => Type::nonNull(Type::id()),
223+
],
224+
'resolve' => static fn ($root, array $args): string => 'node-' . $args['id'],
225+
],
226+
],
227+
]);
228+
229+
$types = ['Query' => $queryType, 'ID' => $customID];
230+
231+
$schema = new Schema([
232+
'query' => $queryType,
233+
'typeLoader' => static fn (string $name): ?Type => $types[$name] ?? null,
234+
]);
235+
236+
$result = GraphQL::executeQuery($schema, 'query ($id: ID!) { node(id: $id) }', null, null, ['id' => 'abc-123']);
237+
238+
self::assertEmpty($result->errors, isset($result->errors[0]) ? $result->errors[0]->getMessage() : '');
239+
self::assertSame(['data' => ['node' => 'node-abc-123']], $result->toArray());
240+
}
241+
242+
public function testTypeLoaderOverrideWithNullableVariableOfOverriddenBuiltInScalarType(): void
243+
{
244+
$customString = self::createUppercaseString();
245+
246+
$queryType = new ObjectType([
247+
'name' => 'Query',
248+
'fields' => [
249+
'echo' => [
250+
'type' => Type::string(),
251+
'args' => [
252+
'text' => Type::string(),
253+
],
254+
'resolve' => static fn ($root, array $args): ?string => $args['text'] ?? null,
255+
],
256+
],
257+
]);
258+
259+
$types = ['Query' => $queryType, 'String' => $customString];
260+
261+
$schema = new Schema([
262+
'query' => $queryType,
263+
'typeLoader' => static fn (string $name): ?Type => $types[$name] ?? null,
264+
]);
265+
266+
$result = GraphQL::executeQuery($schema, 'query ($text: String) { echo(text: $text) }', null, null, ['text' => 'hello']);
267+
268+
self::assertEmpty($result->errors, isset($result->errors[0]) ? $result->errors[0]->getMessage() : '');
269+
self::assertSame(['data' => ['echo' => 'HELLO']], $result->toArray());
270+
}
271+
207272
/** @throws InvariantViolation */
208273
private static function createUppercaseString(): CustomScalarType
209274
{

0 commit comments

Comments
 (0)