Skip to content

fix: restore lenient variable coercion for built-in scalars in GraphQL runtime #655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vsseixaso wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: restore lenient variable coercion for built-in scalars in GraphQL runtime #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Fixed
- GraphQL: restore lenient variable coercion for the built-in `Int`, `Float`, and
`String` scalars. graphql-js 14.5+ (bundled since `@vtex/api@6`) made variable
coercion strict, breaking apps that migrated from `node@4.x` (`@vtex/api@^3`,
graphql 0.13). Incoming variables are now pre-coerced before `execute()` so that
string numbers map to `Int`/`Float` and primitive values map to `String`,
matching the pre-14.5 behavior. Disable with
`VTEX_API_DISABLE_LENIENT_VARIABLE_COERCION=true`.

## [7.3.1] - 2026-01-06

Expand Down
4 changes: 3 additions & 1 deletion src/service/worker/runtime/graphql/middlewares/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { execute } from 'graphql'

import { ExecutableSchema, GraphQLServiceContext } from '../typings'
import { coerceVariableValuesLenient } from '../utils/coerceVariablesLenient'

export const run = (executableSchema: ExecutableSchema) =>
async function runHttpQuery(ctx: GraphQLServiceContext, next: () => Promise<void>) {
Expand All @@ -10,14 +11,15 @@ export const run = (executableSchema: ExecutableSchema) =>

const { document, operationName, variables: variableValues } = query!
const schema = executableSchema.schema
const coercedVariables = coerceVariableValuesLenient(schema, document, variableValues, operationName)
const response = await execute({
contextValue: ctx,
document,
fieldResolver: (root, _, __, info) => root[info.fieldName],
operationName,
rootValue: null,
schema,
variableValues,
variableValues: coercedVariables,
Copy link
Copy Markdown

@mendescamara mendescamara May 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vamos precisar testar, mas acho que esse approach não vai contemplar os casos de variáveis não literais inline, quando o valor é informado diretamente na Query sem usar o parâmetro variables 🤔 (ex. user(id: "123"))

Diferente de quando passa por variável:

// A query permanece estática (não muda)
const GET_USER = `
  query GetUser($userId: ID!) {
    user(id: $userId) {
      name
    }
  }
`;

// Os valores são passados separadamente
const variables = { userId: "123" };

client.query({ query: GET_USER, variables });

Copy link
Copy Markdown

@mendescamara mendescamara May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um outro approach seria fazer o Override dos Scalars Nativos via Schema Transform

Após makeExecutableSchema em schema/index.ts, manipular o _typeMap da schema para substituir os scalars built-in

Copy link
Copy Markdown
Contributor Author

@vsseixaso vsseixaso May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validei aqui e inline literal não é coberto mesmo. E também o inline já era rejeitado no graphql 0.13, peguei os erros do log de prod e todos passam as variáveis pelo objeto variables, nenhum usa inline literal. Então o escopo atual resolve os casos reais, sem regressão.

Fiz um teste mandando uma query com inline literal pra mostrar pra identificar a diferença na mensagem de erro:

{
  "errors": [{
    "message": "Expected type Int, found \"20\".",
    "stack": "GraphQLError: Expected type Int, found \"20\".\n    at isValidScalar (.../ValuesOfCorrectType.js:159)\n    at validate (.../validate.js:73)\n    at parseAndValidateQueryToSchema (.../query.js:28)"
  }]
}

Erro quando vem com Variable:

{
  "errors": [{
    "message": "Variable \"$categoryId\" got invalid value \"20\"; Expected type Int. Int cannot represent non-integer value: \"20\"",
    "stack": "TypeError: Int cannot represent non-integer value: \"20\"\n    at GraphQLScalarType.coerceInt [as parseValue] (.../scalars.js:55)\n    at coerceInputValueImpl (.../coerceInputValue.js:127)\n    at coerceVariableValues (.../values.js:119)\n    at runHttpQuery (.../run.js:9)"
  }]
}

No caso, a decisão fica sendo sobre querer suportar algo além do graphql 0.13 ou se fazer "Override dos Scalars Nativos via Schema Transform" teria algum ganho adicional

Copy link
Copy Markdown

@mendescamara mendescamara May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Top! Então na versão antiga (graphql 0.13) já tinha essa proteção para o inline?

To super confortável de seguir com esse approach 👊🏻

})
ctx.graphql.graphqlResponse = response

Expand Down
Loading