Skip to content

feat: scalar coercion via resolver map override (Approach 1 — Global) - #691

Closed
mendescamara wants to merge 1 commit into
masterfrom
feat/global-scalar-coercion
Closed

feat: scalar coercion via resolver map override (Approach 1 — Global)#691
mendescamara wants to merge 1 commit into
masterfrom
feat/global-scalar-coercion

Conversation

@mendescamara

@mendescamara mendescamara commented May 6, 2026

Copy link
Copy Markdown
Contributor

Context

GraphQL's built-in scalars (Int, Float, String) have strict parseValue and parseLiteral implementations. When clients or upstream systems send values in a compatible but different format — e.g. "20" instead of 20 for an Int field — the operation fails at the scalar validation layer.

This PR explores Approach 1: global interception by replacing the built-in scalars in the schema's type map.

How it works

graphql-tools v3.x (makeExecutableSchema) processes the resolver map via addResolversToSchema. When it finds a GraphQLScalarType instance (not just a plain object) registered under a built-in scalar name (Int, Float, String), it replaces the entry in schema._typeMap[name] — the schema's own private type registry. This is safe because:

  • It does not mutate the global GraphQLInt/GraphQLFloat/GraphQLString singletons from the graphql package.
  • The replacement is scoped to this schema instance only.
  • serialize is delegated unchanged to the original scalar, so output coercion is not affected.

Changes

node/scalars/index.ts (new)

Three GraphQLScalarType instances named Int, Float, and String:

Scalar parseValue behavior parseLiteral addition
Int parseInt(String(value)) — accepts "20", 20.9 Also handles Kind.STRING literals
Float parseFloat(String(value)) — accepts "3.14", 3 Also handles Kind.STRING, Kind.INT literals
String String(value) — accepts numbers, booleans Also handles Kind.INT, Kind.FLOAT, Kind.BOOLEAN literals

node/resolvers/index.ts

Adds Int, Float, String entries pointing to the coercible scalar instances.

Trade-offs

Approach 1 (this PR)
Scope All fields in the entire schema automatically
SDL changes None required
Risk Removes strict type validation globally — e.g. a malformed "abc" for an Int throws at runtime instead of schema validation time
Maintenance Single file to update

Compare with Approach 2 — @coerce directive which applies coercion surgically per field.

References

  • graphql-tools v3.x addResolversToSchema — handles GraphQLScalarType instances by replacing schema._typeMap[name]
  • node-vtex-api @sanitize directive — established precedent of replacing scalar types at schema-build time
  • graphql@0.13.2 scalar resolution flow: parseLiteral for inline values, parseValue for variables

🤖 Generated with Claude Code

@mendescamara @claude
feat: override built-in scalars to coerce Int, Float and String inputs
0edd893 
graphql-tools v3.x replaces schema._typeMap entries when a GraphQLScalarType
instance is provided in the resolver map, safely overriding Int/Float/String
without mutating the global graphql package singletons.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mendescamara
mendescamara requested a review from a team as a code owner May 6, 2026 13:34
@mendescamara
mendescamara requested review from RodrigoTadeuF, gabpaladino and leo-prange-vtex and removed request for a team May 6, 2026 13:34
@vtex-io-ci-cd

vtex-io-ci-cd Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

  • Patch (backwards-compatible bug fixes)

  • Minor (backwards-compatible functionality)

  • Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

  • No thanks, I would rather do it manually 😞

@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Fails
🚫

We follow keepachangelog standards.

Please, change the file CHANGELOG.md adding a small summary of your changes under the [Unreleased] section. Example:

 ## [Unreleased]

+ ### Fixed
+ - Issue with image sizing in the SKU Selector.

Possible types of changes:

  • Added for new features.
  • Changed for changes in existing functionality.
  • Deprecated for soon-to-be removed features.
  • Removed for now removed features.
  • Fixed for any bug fixes.
  • Security in case of vulnerabilities.

Generated by 🚫 dangerJS against 0edd893

mendescamara added a commit that referenced this pull request May 7, 2026
@mendescamara @claude
feat: @Coerce schema directive for surgical per-field scalar coercion…
ce0829b 
… (Approach 2) (#692)

## Context

GraphQL's built-in scalars (`Int`, `Float`, `String`) reject values that
are technically compatible but typed differently — e.g. `"20"` for an
`Int` argument. A global override (see [Approach 1](../pull/691))
removes strict validation for the entire schema, which may be
undesirable.

This PR explores **Approach 2: surgical coercion** via a `@coerce`
schema directive that can be applied selectively to individual
`INPUT_FIELD_DEFINITION` or `ARGUMENT_DEFINITION` locations.

## How it works

The directive follows the exact same pattern as [`@sanitize` in
node-vtex-api](https://github.com/vtex/node-vtex-api/blob/master/src/service/worker/runtime/graphql/schema/schemaDirectives/sanitize.ts):
a `SchemaDirectiveVisitor` subclass that **replaces the field's scalar
type at schema-build time** with a coercible version.

```
makeExecutableSchema
  → applySchemaDirectives
    → Coerce.visitInputFieldDefinition(field)
      → field.type = replaceWithCoercible(field.type)   // swap Int → CoercibleInt
```

`replaceWithCoercible` walks `GraphQLNonNull` and `GraphQLList` wrappers
recursively to reach the inner named scalar, then swaps it with the
matching coercible instance. Only `Int`, `Float`, and `String` have
coercible versions; other types pass through unchanged.

## Changes

### `node/directives/coerce.ts` (new)
- `Coerce extends SchemaDirectiveVisitor` with
`visitInputFieldDefinition` and `visitArgumentDefinition`
- Three coercible scalar instances (same logic as Approach 1, but scoped
to this directive)
- `replaceWithCoercible` helper that unwraps `NonNull`/`List` before
substituting

### `node/directives/index.ts`
Registers `coerce: Coerce` in the `schemaDirectives` map.

### `graphql/schema.graphql`
Declares the directive:
```graphql
directive @Coerce on INPUT_FIELD_DEFINITION | ARGUMENT_DEFINITION
```

### `graphql/types/Shipping.graphql`
Applies the directive as a concrete example:
```graphql
# Before
input ShippingItem {
  quantity: String   # typed as String for legacy reasons
}

# After
input ShippingItem {
  quantity: Int @Coerce   # semantically correct, still accepts "20"
}
```
`ShippingItem.quantity` was previously typed as `String` even though it
represents an integer count. With `@coerce` it becomes `Int` (correct
semantic type) while retaining backward compatibility for clients that
send it as a string.

## Trade-offs

| | Approach 2 (this PR) |
|---|---|
| **Scope** | Only fields explicitly annotated with `@coerce` |
| **SDL changes** | Directive declaration + annotation per field |
| **Risk** | Minimal — strict validation preserved everywhere else |
| **Maintenance** | Each new field needs the annotation |

> Compare with [Approach 1 — global scalar override](../pull/691) which
coerces all fields automatically.

## References
- node-vtex-api `@sanitize` directive — `SchemaDirectiveVisitor`
replacing field types at build time
- `graphql-tools` v3.x `SchemaDirectiveVisitor` — supports
`visitInputFieldDefinition` and `visitArgumentDefinition`
- `graphql@0.13.2` scalar resolution flow: `parseLiteral` for inline
values, `parseValue` for variables

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gabpaladino gabpaladino Awaiting requested review from gabpaladino gabpaladino is a code owner automatically assigned from vtex-apps/store-framework-devs
@RodrigoTadeuF RodrigoTadeuF Awaiting requested review from RodrigoTadeuF RodrigoTadeuF is a code owner automatically assigned from vtex-apps/store-framework-devs
@leo-prange-vtex leo-prange-vtex Awaiting requested review from leo-prange-vtex leo-prange-vtex is a code owner automatically assigned from vtex-apps/store-framework-devs

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mendescamara