Remove deprecated ScalarWrapper and scalar class wrapping#4227
Remove deprecated ScalarWrapper and scalar class wrapping#4227Ckk3 wants to merge 9 commits intostrawberry-graphql:mainfrom
ScalarWrapper and scalar class wrapping#4227Conversation
…(NewType(...), ...)
Reviewer's GuideRemove the deprecated strawberry.scalar(cls, ...) wrapper pattern and ScalarWrapper, standardizing scalar handling on ScalarDefinition plus StrawberryConfig.scalar_map, and updating schema/codegen, federation helpers, runtime, tests, and docs accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
c74eb8b to
062d67d
Compare
|
Thanks for adding the Here's a preview of the changelog: Remove deprecated You can run Migration guideBefore (deprecated): import strawberry
from datetime import datetime
EpochDateTime = strawberry.scalar(
datetime,
serialize=lambda v: int(v.timestamp()),
parse_value=lambda v: datetime.fromtimestamp(v),
)
@strawberry.type
class Query:
created: EpochDateTimeAfter: import strawberry
from typing import NewType
from datetime import datetime
from strawberry.schema.config import StrawberryConfig
EpochDateTime = NewType("EpochDateTime", datetime)
@strawberry.type
class Query:
created: EpochDateTime
schema = strawberry.Schema(
query=Query,
config=StrawberryConfig(
scalar_map={
EpochDateTime: strawberry.scalar(
name="EpochDateTime",
serialize=lambda v: int(v.timestamp()),
parse_value=lambda v: datetime.fromtimestamp(v),
)
}
),
)Here's the tweet text: |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
strawberry.federation.scalaryou now constructScalarDefinitiondirectly withorigin=None, which means federated scalars will no longer carry source file/line information used by rich exceptions and locate-definition; consider mirroring the mainscalarimplementation by capturing_source_file/_source_lineand settingoriginso tooling behaves consistently for federated scalars.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `strawberry.federation.scalar` you now construct `ScalarDefinition` directly with `origin=None`, which means federated scalars will no longer carry source file/line information used by rich exceptions and locate-definition; consider mirroring the main `scalar` implementation by capturing `_source_file` / `_source_line` and setting `origin` so tooling behaves consistently for federated scalars.
## Individual Comments
### Comment 1
<location> `strawberry/schema/schema_converter.py:810-811` </location>
<code_context>
scalar_definition: ScalarDefinition
if scalar in self.scalar_registry:
- _scalar_definition = self.scalar_registry[scalar]
- # TODO: check why we need the cast and we are not trying with getattr first
- if isinstance(_scalar_definition, ScalarWrapper):
- scalar_definition = _scalar_definition._scalar_definition
- else:
- scalar_definition = _scalar_definition
+ scalar_definition = self.scalar_registry[scalar]
else:
scalar_definition = scalar._scalar_definition # type: ignore[attr-defined]
</code_context>
<issue_to_address>
**issue (bug_risk):** Fallback to `scalar._scalar_definition` is now effectively dead/unsafe with the new scalar API.
Since `ScalarWrapper` is gone and `scalar()` no longer attaches `_scalar_definition`, this `else` branch now relies on an attribute the new API never sets. Legacy decorated scalars (or any type without `_scalar_definition`) will now raise `AttributeError` rather than a clear migration error.
Consider either:
- Detecting this case explicitly and raising a clear deprecation/migration error (e.g. `TypeError` explaining decorator-style scalars are no longer supported), or
- Removing the `else` branch and only supporting scalars present in `scalar_registry`.
This would prevent unexpected runtime errors and make the new scalar requirements explicit.
</issue_to_address>
### Comment 2
<location> `docs/types/schema.md:113` </location>
<code_context>
-#### `scalar_overrides: Optional[Dict[object, ScalarWrapper]] = None`
+#### `scalar_overrides: Optional[Dict[object, ScalarDefinition]] = None`
Override the implementation of the built in scalars.
[More information](/docs/types/scalars#overriding-built-in-scalars).
</code_context>
<issue_to_address>
**nitpick (typo):** Use hyphenated "built-in" for consistency and correct usage.
Update this sentence to use "built-in" so it matches the later "built-in scalars" link text.
```suggestion
Override the implementation of the built-in scalars.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Greptile SummaryRemoves the deprecated Key changes:
Migration pattern: Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User defines custom scalar] --> B{Old Pattern}
B --> C[strawberry.scalar wraps type]
C --> D[ScalarWrapper created]
D --> E[Schema uses wrapper]
A --> F{New Pattern}
F --> G[NewType definition]
G --> H[scalar returns ScalarDefinition]
H --> I[Add to scalar_map in config]
I --> J[Schema uses ScalarDefinition]
K[Schema Codegen] --> L[Parse GraphQL schema]
L --> M[Generate NewType for custom scalars]
M --> N[Generate scalar_map config]
N --> O[Output complete schema code]
P[Federation Scalars] --> Q[Define NewType]
Q --> R[federation.scalar with directives]
R --> S[Add to scalar_map]
S --> T[Directives applied to scalar]
style D fill:#ffcccc
style E fill:#ffcccc
style C fill:#ffcccc
style J fill:#ccffcc
style O fill:#ccffcc
style T fill:#ccffcc
Last reviewed commit: 444ad2e |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
query_codegen._get_field_type, when a scalar registry key is neither atypenor has__supertype__,python_typeends up asNoneand is passed into_collect_scalar; consider either constraining the allowed scalar registry keys or making_collect_scalarrobust to aNonepython_typeto avoid subtle failures. - The new
federation.scalarimplementation always setsorigin=Noneon the returnedScalarDefinition, which drops the source information you collect instrawberry.types.scalar.scalar; consider capturing the caller frame here as well so rich error reporting remains consistent for federation scalars.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `query_codegen._get_field_type`, when a scalar registry key is neither a `type` nor has `__supertype__`, `python_type` ends up as `None` and is passed into `_collect_scalar`; consider either constraining the allowed scalar registry keys or making `_collect_scalar` robust to a `None` `python_type` to avoid subtle failures.
- The new `federation.scalar` implementation always sets `origin=None` on the returned `ScalarDefinition`, which drops the source information you collect in `strawberry.types.scalar.scalar`; consider capturing the caller frame here as well so rich error reporting remains consistent for federation scalars.
## Individual Comments
### Comment 1
<location> `strawberry/federation/scalar.py:139-148` </location>
<code_context>
- def wrap(cls: _T) -> ScalarWrapper:
- return _process_scalar(
- cls,
+ if name is not None:
+ return ScalarDefinition(
name=name,
description=description,
specified_by_url=specified_by_url,
serialize=serialize,
- parse_value=parse_value,
parse_literal=parse_literal,
- directives=directives,
+ parse_value=parse_value,
+ directives=tuple(all_directives),
+ origin=None,
)
- if cls is None:
- return wrap
+ # Decorator pattern for type hinting purposes only
+ def wrap(cls: _T) -> _T:
</code_context>
<issue_to_address>
**issue (bug_risk):** Decorator form of `federation.scalar` now ignores all runtime options and silently becomes a no-op.
The updated behavior means decorator usage no longer creates a `ScalarDefinition`, so federation options like `authenticated`, `inaccessible`, `policy`, `requires_scopes`, `tags`, or `directives` are ignored when `name` is omitted. To avoid silent behavior changes, consider either raising when `name is None` and any of these options are passed, or preserving the previous decorator behavior that created a `ScalarDefinition` for backward compatibility.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ) | ||
| from typing import NewType | ||
|
|
||
| BigInt = NewType("BigInt", int) |
There was a problem hiding this comment.
we need to define the scalar for this 😊
docs/integrations/pydantic.md
Outdated
| scalar_overrides={ | ||
| Base64: strawberry.scalar( | ||
| name="Base64", | ||
| serialize=lambda v: base64.b64encode(v).decode("utf-8"), | ||
| parse_value=lambda v: base64.b64decode(v.encode("utf-8")), | ||
| ) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
should this be in scalar map instead?

Description
Remove the deprecated
strawberry.scalar(cls, ...)wrapper pattern andScalarWrapper, deprecated since 0.288.0.You can run
strawberry upgrade replace-scalar-wrappers <path>to automatically replace built-in scalar wrapper imports.Migration guide
Before (deprecated):
After:
Types of Changes
Checklist
Summary by Sourcery
Remove the deprecated scalar class wrapper API in favor of ScalarDefinition-based configuration and scalar_map usage across the codebase.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: