fix(@nestjs/graphql): run plugin refresh hooks in registration order#3970
Merged
kamilmysliwiec merged 1 commit intoMay 1, 2026
Merged
Conversation
Nested mapped-type helpers (e.g. PartialType(OmitType(Foo, ['x']))) registered their refresh hooks innermost-first, but MetadataLoader was running them in reverse via Array.unshift. The outer wrapper's hook fired before the inner wrapper had propagated the SWC plugin's emitted fields, so every plugin-only field disappeared from the composed GraphQL type. Switch to Array.push so inner hooks run first and the outer wrapper reads the already-refreshed inner fields. The legacy TS plugin was unaffected because it injects per-class __SCHEMA_METADATA_FACTORY synchronously at class-declaration time, before the helpers run. Closes nestjs#3313
Member
|
lgtm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #3313
Composing mapped-type helpers (e.g.
PartialType(OmitType(Foo, ['x']))) loses every field that comes from the SWC plugin'sSERIALIZED_METADATAemission (i.e. fields declared without an explicit@Field()decorator). The resulting GraphQL type ends up empty or partial. The legacy TS plugin is unaffected.Root cause:
MetadataLoader.addRefreshHookwas usingArray.unshift, so refresh hooks ran in reverse-of-registration order. Each mapped-type helper (PartialType,OmitType,PickType,IntersectionType) registers a hook that re-reads its source class's fields and re-applies them to its wrapper class. When nested — e.g.PartialType(OmitType(CreateInput, ['x']))— the inner helper's hook is registered first, the outer's second. Withunshift, the outer hook fired before the inner wrapper had propagated the SWC plugin's emitted fields, so plugin-only fields disappeared from the composed type. The legacy TS plugin was unaffected because it injects per-class__SCHEMA_METADATA_FACTORYsynchronously at class-declaration time, before the helpers run.What is the new behavior?
MetadataLoader.addRefreshHooknow appends withArray.pushinstead of prepending withArray.unshift, so refresh hooks run in registration order (innermost first). The outer wrapper now reads the inner wrapper's fields after they've been refreshed by the SWC plugin's metadata, and nested compositions correctly carry every plugin-emitted field through to the composed GraphQL type.Does this PR introduce a breaking change?
This change reverses an internal hook execution order that was incorrect for nested compositions; non-nested usage was already correct because there's only one hook to fire. No public API change.
Other information
3 new regression tests added under
packages/graphql/tests/plugin/type-helpers/nested-mapped-types.spec.tscovering:PartialType(OmitType(...))PartialType(PickType(...))IntersectionType(PickType(...), PickType(...))All three fail without the fix and pass with it. Verified locally with
yarn test:e2e:graphql— 210 passed; the 2 unrelated CRLF Windows snapshot failures (readonly-visitor,model-class-visitor) are pre-existing on master and unrelated to this change.Closes #3313