Skip to content

fix(@nestjs/graphql): de-duplicate per-target metadata in TargetMetadataCollection#3960

Merged
kamilmysliwiec merged 1 commit intonestjs:masterfrom
yogeshwaran-c:fix/target-metadata-idempotent-set
Apr 28, 2026
Merged

fix(@nestjs/graphql): de-duplicate per-target metadata in TargetMetadataCollection#3960
kamilmysliwiec merged 1 commit intonestjs:masterfrom
yogeshwaran-c:fix/target-metadata-idempotent-set

Conversation

@yogeshwaran-c
Copy link
Copy Markdown
Contributor

Summary

  • Make TargetMetadataCollection's objectType, argumentType, inputType and resolver setters idempotent: when the same target's metadata is re-assigned, replace the entry in the canonical all.<kind> array in place instead of pushing a second one.

Bug

@ObjectType() registers metadata both eagerly (so getObjectTypeMetadataByTarget(...) works synchronously inside @Resolver(() => Cat)) and again through LazyMetadataStorage.store(...). Each registration goes through:

set objectType(val: ObjectTypeMetadata) {
  this._objectType = val;
  this.all.objectType.push(val);
}

So every class decorated with @ObjectType() (or @ArgsType(), which also pairs an eager + lazy registration) ended up listed twice in getObjectTypesMetadata(). The downstream generateObjectTypeDefs then ran the type-definition factory once per duplicate. addObjectTypes (Map.set) hid the resulting collision but the per-target metadata compilation, plugin metadata loading, and orphan walks still all ran twice.

The same shape applies to @ArgsType() (also eager + lazy) and to @InputType() once PickType (or other helpers) double-decorate a generated abstract class.

Fix

The setters now look up the previous value in the canonical array and replace it in place, falling back to a push only when the target has not been registered yet:

private replaceOrPush<T>(list: T[], previous: T | undefined, next: T) {
  if (previous === undefined) { list.push(next); return; }
  const index = list.indexOf(previous);
  if (index === -1) list.push(next); else list[index] = next;
}

This keeps the all.<kind> arrays as a one-entry-per-target list while still exposing the most recent metadata reference. interface already uses Map.set keyed by target and is unchanged.

Test plan

  • New regression test tests/schema-builder/storages/target-metadata-set.spec.ts (fails on master; passes after the fix) covers @ObjectType and @ArgsType.
  • packages/graphql Vitest run is green (the pre-existing model-class-visitor / readonly-visitor plugin snapshot failures already exist on master).
  • packages/apollo e2e run (tests/e2e) passes including serialized-graph.

…ataCollection

`@ObjectType()` (and `@ArgsType()`) call `addObjectTypeMetadata` /
`addArgsMetadata` eagerly so resolvers can read the type name
synchronously, and again through `LazyMetadataStorage.store(...)` so the
metadata is rebuildable. The collection's `set objectType` /
`set argumentType` / `set inputType` / `set resolver` accessors blindly
pushed onto the canonical `all.<kind>` arrays each time, so every class
ended up listed twice and the schema generator processed it twice.

Replace the value in place when the same target is re-set so the
canonical list always contains exactly one entry per target while still
exposing the latest metadata reference.
@kamilmysliwiec kamilmysliwiec merged commit 6aa3166 into nestjs:master Apr 28, 2026
1 check passed
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