Stop calling typeLoader for built-in scalar names#1884
Stop calling typeLoader for built-in scalar names#1884
Conversation
Calling the typeLoader for built-in scalar names (String, Int, Float, Boolean, ID) breaks downstream consumers whose typeLoaders were not designed to handle these names. Remove typeLoader-based scalar override support, keeping only types-based overrides. Guard loadType() so the typeLoader is never invoked for built-in scalar names. Fixes #1874 🤖 Generated with Claude Code
|
How is this now going to be supported when using a type loader? With this Pr you can only do it when using a types map? |
|
I'd rather have an option that enables scalars via type loader. Make the default behavior in a new version. But it allows people to opt-in when using a type loader. Also, I have the feeling that people that want custom scalar types, often also have a type loader. So making this only apply to people without a type loader does not make much sense to me. |
|
So basically I would vote for #1882 |
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 89cd752 | Previous: 317a71e | Ratio |
|---|---|---|---|
DeferredBench::benchManyNestedDeferreds |
21.75 ms |
12.163 ms |
1.79 |
This comment was automatically generated by workflow using github-action-benchmark.
🤖 Generated with Claude Code
…calar overrides 🤖 Generated with Claude Code
|
typeLoader users can still override built-in scalars — they just use $schema = new Schema([
'query' => $queryType,
'typeLoader' => $myTypeLoader,
'types' => [$uppercaseString],
]);I added a test that proves both mechanisms work together: the typeLoader resolves custom types (like Also added documentation for this in the scalars docs and updated the |
|
Oké great let's try it out! |
Summary
Fixes #1874
Calling the
typeLoaderfor built-in scalar names (String,Int,Float,Boolean,ID) breaks downstream consumers whose typeLoaders were not designed to handle these names.This removes typeLoader-based scalar override support, keeping only
types-based overrides.getTypeMap()that iterated built-in scalar namesloadType()against built-in scalar names so the typeLoader is never called for themtypesconfigTest plan
vendor/bin/phpunit tests/Type/ScalarOverridesTest.php— all 11 tests passvendor/bin/phpunit— full suite green (1 pre-existing unrelated failure in EnumTypeTest)vendor/bin/phpstan— no errors🤖 Generated with Claude Code