Fix type loader scalar overrides breaking validation and input coercion by ruudk · Pull Request #1875 · webonyx/graphql-php

ruudk · 2026-03-16T10:58:55Z

When a type loader returns a custom instance for a built-in scalar (e.g. ID, String), two problems occur because field definitions still reference the built-in singletons (e.g. Type::id()):

Variable validation fails

VariablesInAllowedPosition resolves variable types via Schema::getType(), which returns the type loader's instance. TypeComparators::isEqualType() and isTypeSubTypeOf() compare this against field argument types using strict identity (===). Since the type loader instance differs from the built-in singleton, the check fails with errors like:

Variable "$id" of type "ID!" used in position expecting type "ID!".

Fix: add a name-based equality check for NamedType instances in both isEqualType() and isTypeSubTypeOf(). 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.
Input coercion uses wrong parseValue()

Value::coerceInputValue() calls parseValue() on the type from the field definition (the built-in singleton), not the custom override from the type loader. This means custom parsing logic (e.g. converting a base64 global ID string into a domain object) is skipped, causing input validation to fail.

This is the input-side equivalent of the output fix already present in ReferenceExecutor::completeValue() (lines 920-923).

Fix: add an optional Schema parameter to coerceInputValue(). When provided and the type is a ScalarType, resolve the actual type from the schema to ensure the correct parseValue() is called. The schema is passed from Values::getVariableValues() and propagated through all recursive calls.

Ref #1869
Fixes #1874

ruudk · 2026-03-16T11:10:15Z

OK, PR updated with both fixes. Confirmed that it works on our large GraphQL server with custom ID type.

spawnia · 2026-03-16T11:25:10Z

@ruudk Can you create a branch directly in this project? Makes it easier to install for testing (no extra repo config necessary).

Copilot

Pull request overview

Fixes per-schema built-in scalar overrides (via typeLoader) so they don’t break variable type validation and ensure overridden scalars’ parseValue() is used during variable/input coercion.

Changes:

Treat NamedTypes with the same name as equal/subtypes in TypeComparators to avoid identity (===) mismatches from scalar overrides.
Extend Value::coerceInputValue() to optionally take a Schema and resolve overridden scalar instances from the schema before calling parseValue().
Propagate the schema into variable coercion (Values::getVariableValues) and add regression tests for variable validation + input object coercion.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/Type/ScalarOverridesTest.php	Adds regression tests covering overridden built-in scalars with variables and input objects.
src/Utils/Value.php	Adds optional `Schema` parameter to resolve overridden scalar instances for correct `parseValue()` during input coercion.
src/Utils/TypeComparators.php	Adds name-based equality/subtyping for `NamedType` instances to avoid identity mismatch failures.
src/Executor/Values.php	Passes schema into `Value::coerceInputValue()` during variable coercion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

tests/Type/ScalarOverridesTest.php

spawnia · 2026-03-16T11:36:45Z

@ruudk Really appreciate you looking into this! I can't look into it too deeply for now given my work responsibilities for the day, will try to tend to it as much as i can though.

ruudk · 2026-03-16T11:40:34Z

I'll push a branch to this repo next time. I think if I change it now it might close this PR.

I will apply the feedback and add more tests.

Then I would suggest to merge this and I will switch our project to use main on production. If that works, it's ready to release.

Copilot

Pull request overview

This PR fixes issues introduced when a schema’s type loader returns custom instances for built-in scalars (e.g., ID, String), where validation and input coercion previously relied on strict object identity and the built-in singleton instances.

Changes:

Make TypeComparators::isEqualType() and isTypeSubTypeOf() treat NamedTypes with the same name as equivalent, avoiding identity-based mismatches during validation.
Ensure variable input coercion uses the schema-resolved scalar instance (and its parseValue()), by threading an optional Schema through Value::coerceInputValue() from Values::getVariableValues().
Add regression tests covering variable validation and input object coercion with type-loader scalar overrides.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

File	Description
tests/Type/ScalarOverridesTest.php	Adds regression tests for variable validation and input coercion when built-in scalars are overridden via `typeLoader`.
src/Utils/Value.php	Adds optional `Schema` parameter to resolve schema scalar instances for correct `parseValue()` during input coercion.
src/Utils/TypeComparators.php	Adds name-based equality for `NamedType` instances to avoid strict-identity failures in validation.
src/Executor/Values.php	Passes schema into `Value::coerceInputValue()` during variable coercion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

tests/Type/ScalarOverridesTest.php

+            'parseLiteral' => static function ($node): string {
+                if (! $node instanceof StringValueNode) {
+                    throw new \Exception('Expected a string literal for ID.');
+                }
+
+                return $node->value;
+            },


tests/Type/ScalarOverridesTest.php

+            'parseLiteral' => static function ($node): string {
+                if (! $node instanceof StringValueNode) {
+                    throw new \Exception('Expected a string literal for String.');
+                }
+
+                return $node->value;
+            },


ruudk · 2026-03-16T12:02:28Z

src/Utils/TypeComparators.php

+        // Named types with the same name are equal, even if they are different
+        // instances (e.g. a type loader override vs the built-in singleton).
+        if ($typeA instanceof NamedType && $typeB instanceof NamedType
+            && $typeA->name() === $typeB->name()
+        ) {
+            return true;
+        }


It should only apply to scalar types.

When a type loader returns a custom instance for a built-in scalar (e.g. ID, String), two problems occur because field definitions still reference the built-in singletons (e.g. Type::id()): 1. Variable validation fails VariablesInAllowedPosition resolves variable types via Schema::getType(), which returns the type loader's instance. TypeComparators::isEqualType() and isTypeSubTypeOf() compare this against field argument types using strict identity (===). Since the type loader instance differs from the built-in singleton, the check fails with errors like: Variable \"$id\" of type \"ID!\" used in position expecting type \"ID!\". Fix: add a name-based equality check for NamedType instances in both isEqualType() and isTypeSubTypeOf(). 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. 2. Input coercion uses wrong parseValue() Value::coerceInputValue() calls parseValue() on the type from the field definition (the built-in singleton), not the custom override from the type loader. This means custom parsing logic (e.g. converting a base64 global ID string into a domain object) is skipped, causing input validation to fail. This is the input-side equivalent of the output fix already present in ReferenceExecutor::completeValue() (lines 920-923). Fix: add an optional Schema parameter to coerceInputValue(). When provided and the type is a ScalarType, resolve the actual type from the schema to ensure the correct parseValue() is called. The schema is passed from Values::getVariableValues() and propagated through all recursive calls. Ref webonyx#1869 Fixes webonyx#1874

ruudk · 2026-03-16T12:05:29Z

I'm going to do a different approach.

ruudk requested a review from spawnia March 16, 2026 10:58

ruudk mentioned this pull request Mar 16, 2026

After Upgrading from v15.30.2 => v15.31.0 Error: " {"exception":"[object] (GraphQL\\Error\\Error(code: 0): Config element \"String\" is not declared in GraphQL schema at " #1874

Open

This comment was marked as outdated.

Sign in to view

ruudk force-pushed the fix-built-in-scalars-comparison branch from 85a759b to b66386f Compare March 16, 2026 11:08

ruudk changed the title ~~Fix variable validation failing when type loader overrides built-in scalars~~ Fix type loader scalar overrides breaking validation and input coercion Mar 16, 2026

spawnia requested a review from Copilot March 16, 2026 11:20

Copilot started reviewing on behalf of spawnia March 16, 2026 11:20 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

tests/Type/ScalarOverridesTest.php Outdated Show resolved Hide resolved

tests/Type/ScalarOverridesTest.php Outdated Show resolved Hide resolved

ruudk force-pushed the fix-built-in-scalars-comparison branch from 93f8f3b to 26ce8fc Compare March 16, 2026 11:52

ruudk requested a review from Copilot March 16, 2026 11:54

Copilot started reviewing on behalf of ruudk March 16, 2026 11:54 View session

ruudk force-pushed the fix-built-in-scalars-comparison branch from 26ce8fc to 3cf494b Compare March 16, 2026 11:57

Copilot AI reviewed Mar 16, 2026

View reviewed changes

ruudk force-pushed the fix-built-in-scalars-comparison branch from 3cf494b to a7e0f7e Compare March 16, 2026 12:00

ruudk closed this Mar 16, 2026

ruudk deleted the fix-built-in-scalars-comparison branch March 16, 2026 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix type loader scalar overrides breaking validation and input coercion#1875

Fix type loader scalar overrides breaking validation and input coercion#1875
ruudk wants to merge 1 commit intowebonyx:masterfrom
ruudk:fix-built-in-scalars-comparison

ruudk commented Mar 16, 2026 •

edited

Loading

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ruudk commented Mar 16, 2026

Uh oh!

spawnia commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

spawnia commented Mar 16, 2026

Uh oh!

ruudk commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

ruudk Mar 16, 2026

Uh oh!

ruudk commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ruudk commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ruudk commented Mar 16, 2026

Uh oh!

spawnia commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

spawnia commented Mar 16, 2026

Uh oh!

ruudk commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ruudk Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

ruudk commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ruudk commented Mar 16, 2026 •

edited

Loading