Repository navigation
fix(codegen): allow custom scalars from scalar_map without explicit python type (#4741) - #4789
Rajan-vish wants to merge 3 commits into
Conversation
|
Thanks for adding the Below is the changelog that will be used for the release. This release fixes an issue where running
This release was contributed by @Rajan-vish in #4789 |
[Medium impact] The changes affect the bounded workflow that generates Python and TypeScript client types. Fix duplicate TypeScript aliases and undefined FindingsSummaryFixes custom scalar code generation by retaining Python types from
|
| not isinstance(field_type, StrawberryType) | ||
| and field_type in self.schema.schema_converter.scalar_registry | ||
| ): | ||
| original_python_type = field_type |
There was a problem hiding this comment.
Preserving each field's Python type can generate the same TypeScript alias twice. Mapping str and float to one scalar definition named Custom is accepted by the schema converter. Selecting fields of both types now collects two different GraphQLScalar objects and emits both type Custom = string and type Custom = number. TypeScript rejects the duplicate name.
Choose one Python type per scalar definition and reuse it for fields and variables.
There was a problem hiding this comment.
Addressed in commit 341a604 by caching GraphQLScalar in self._scalar_definitions by scalar name in QueryCodegen._collect_scalar, ensuring each scalar definition is collected once and reused across all fields and variables without duplicate alias generation.
| type_name = type_.python_type.__name__ | ||
| if hasattr( | ||
| type_.python_type, "__module__" | ||
| ) and type_.python_type.__module__ not in ("builtins", "__main__"): | ||
| self.imports[type_.python_type.__module__].add(type_name) |
There was a problem hiding this comment.
Generated Python references undefined
NoneType
A field returning None resolves to the registered Void scalar with python_type=type(None). This branch emits Void = NewType("Void", NoneType) but skips its import because its module is builtins. NoneType is not a built-in name, so importing the generated file raises NameError.
Import NoneType from types, or print type(None) instead.
There was a problem hiding this comment.
Addressed in commit 341a604 by importing NoneType from types when type_.python_type is type(None), and mapping type(None) to null in TypeScript.
There was a problem hiding this comment.
Linked issues
#4741 codegen: allow mapping custom scalars to Python types (`Scalar type must have a python type`) · 1 of 3 met
| # | Requirement | Status | Notes |
|---|---|---|---|
| 1 | Prevent codegen from aborting on custom scalars registered through StrawberryConfig.scalar_map. | ✅ Met | QueryCodegen preserves or recovers the scalar's Python type, and the plugins fall back to Any/any when no type is available. |
| 2 | Allow scalar names to be mapped to Python annotations through the CLI or a config file. | ❌ Not addressed | The change uses types already present in the schema's scalar registry; it adds no CLI or config-file mapping for scalar names to annotations. |
| 3 | Fall back to Any for an unmapped scalar with a warning instead of raising an assertion. | 🟡 Partly met | Python codegen falls back to Any, but it does not emit a warning. |
Description
Fixes #4741.
When custom scalars are mapped via StrawberryConfig(scalar_map={...}) or have ScalarDefinition without an explicit ScalarWrapper, _get_field_type in QueryCodegen was discarding the original Python type and passing None to _collect_scalar. This caused PythonPlugin._print_scalar_type to fail with:
AssertionError: Scalar type must have a python type:
and TypeScriptPlugin to fail with KeyError or assertion errors.
Solution
Types of Changes
Checklist
Summary by Sourcery
Fix client code generation for custom scalars by preserving their underlying Python types and falling back gracefully when type information is unavailable.
Bug Fixes:
Enhancements:
Documentation:
Tests: