Skip to content

fix(@nestjs/graphql): preserve ResolveField options for all overloads#3939

Merged
kamilmysliwiec merged 1 commit intonestjs:masterfrom
maruthang:fix/issue-2587-resolvefield-middleware
Apr 20, 2026
Merged

fix(@nestjs/graphql): preserve ResolveField options for all overloads#3939
kamilmysliwiec merged 1 commit intonestjs:masterfrom
maruthang:fix/issue-2587-resolvefield-middleware

Conversation

@maruthang
Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

(Marking only Bugfix:)

  • Bugfix

What is the current behavior?

Issue Number: #2587

Field middleware configured via @ResolveField({ middleware: [...] }) or @ResolveField('aliased', { middleware: [...] }) is silently dropped. The same middleware fires correctly for plain @Field({ middleware: [...] }) and for @ResolveField(() => SomeType, { middleware: [...] }). The reporter and a co-reporter on the original issue identified that adding an explicit returnType argument to @ResolveField makes the middleware work — that observation is the fingerprint of the underlying decorator-argument-destructuring bug, not a middleware-wiring problem.

What is the new behavior?

packages/graphql/lib/decorators/resolve-field.decorator.ts now branches on the actual shape of the arguments rather than only on isFunction(firstArg). New routing:

(a) first arg function → typeFunc-first overload (unchanged);
(b) first arg string + second arg function → propertyName + typeFunc + options (unchanged);
(c) first arg string + second arg object → propertyName + no-typeFunc + options;
(d) first arg object → no-name + no-typeFunc + options.

Adds an isString import alongside the existing isFunction/isObject from @nestjs/common/utils/shared.utils.

A regression spec at packages/graphql/tests/schema-builder/factories/resolve-field-middleware.spec.ts reads FIELD_RESOLVER_MIDDLEWARE_METADATA, RESOLVER_NAME_METADATA, and RESOLVER_PROPERTY_METADATA straight off the decorated method for three cases (typeFunc + options, options only, propertyName + options) — reading metadata directly avoids requiring an apollo/mercurius runtime to assert the wiring.

Does this PR introduce a breaking change?

  • Yes
  • No

The change only adds correct handling for the previously-broken overloads. Calls that already worked (typeFunc-first or propertyName + typeFunc + options) continue to produce identical metadata.

Other information

Verified with yarn test:e2e:graphql (89 passed plus the unrelated pre-existing Windows CRLF snapshot failures in tests/plugin/model-class-visitor.spec.ts es5-eager-imports and tests/plugin/readonly-visitor.spec.ts, both reproducible on clean master). npx oxlint clean on touched files. Prettier formatting applied with the project's bundled v3 binary (the repo's npx prettier shim resolves to v2 and would reformat unrelated lines).

The argument destructuring in the @ResolveField decorator only branched on whether the
first argument was a function. The else arm assumed the first argument was a string
property name and that the second argument was a typeFunc, so calls of the form
@ResolveField({ middleware: [...] }) put the options object in the name slot and left
the options slot undefined. Calls of the form @ResolveField('aliased', { middleware: [...] })
similarly routed the options object into the typeFunc slot and dropped middleware. The
only consistently-working shape was @ResolveField(typeFunc, { middleware }), which is why
adding an explicit return type appeared to fix the bug.

Extend the destructuring to detect (a) options-only calls when the first argument is a
plain object and (b) propertyName-with-options calls when the second argument is not a
function. The typeFunc-first overloads keep their existing behaviour.

Closes nestjs#2587
@kamilmysliwiec kamilmysliwiec merged commit 36e6396 into nestjs:master Apr 20, 2026
1 check passed
@kamilmysliwiec
Copy link
Copy Markdown
Member

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants