fix(graphql): avoid partial field declarations from inherited metadata with cli plugin

[CarsonF]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

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
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

The former logic was accessing the inherited metadata factory, I guess when the current class didn't have one.
The parent class could have fields that the plugin cannot determine, and are explicitly declared with @Field decorators.
The partial info & the type from decorator are merged together for that base type.
But here when the subclass was redefining this field it only had partial info, no type.
This caused the complication logic to fail, because the typeFn would be missing.
Instead, we only want to look for the own metadata factory, and ignore if there is none.
Just to note: inherited fields are merged elsewhere (in factories), so this doesn't need to try.

A minimal reproduction is

// Base class that the plugin will generate metadata for, but it will be missing
// the `type` because it cannot be resolved by TS.
@ObjectType({ isAbstract: true })
export class Entity {
  @Field(() => String)
  id: unknown;
}

// In the middle is a non-exported / or dynamic class that the plugin ignores
// MetadataLoader will not assign a _GRAPHQL_METADATA_FACTORY property to this object
// because of that.
export function LivingBeing() {
  @ObjectType({ isAbstract: true })
  class LivingBeingClass extends Entity {}
  return LivingBeingClass;
}

// A concrete type that acts as the entrypoint for the bug.
@ObjectType()
export class Person extends LivingBeing() {}

With this example, Person & Entity will have metadata loaded / _GRAPHQL_METADATA_FACTORY property assigned.
The problem is that

LivingBeingClass.constructor[METADATA_FACTORY_NAME]

resolves to Entity's _GRAPHQL_METADATA_FACTORY 💥 , but it's registered as the LivingBeingClass type.
⭐ The logic should be to skip because no _GRAPHQL_METADATA_FACTORY is declared for LivingBeingClass.

Entity's metadata is merged with the decorator, so the plugin doesn't provide the type, but the decorator does.
However, LivingBeingClass does not have this field metadata merged since the Entity.id decorator is not its own (guessing on why).
The factories throw because LivingBeingClass.id does not have a type even though its parent class does.

The metadata should ignore this field for this intermediate class since it doesn't do anything with it - it just inherits.

What is the new behavior?

Error is avoided. Fields are still merged/inherited as expected.

Does this PR introduce a breaking change?

• Yes
• No

Other information

I also added a check to avoid merging metadata for common parents.
i.e. TypeMetadataStorage.addClassFieldMetadata would previous be called for Entity.id for every concrete type extending Entity. Which is redundant work after the first time.

[kamilmysliwiec]

question: any specific reason why we use getOwnPropertyDescriptor now?

[CarsonF]

Yes this is basically the fix. We only want the metadata factory from the referenced class. Static properties (of ES6 classes) are inherited, and we want to avoid this here.
This could be an Object.hasOwn check. I used this to do that check and get the own value at the same time so the constant METADATA_FACTORY_NAME only had to be referenced once.

[CarsonF]

I clarified the PR description. The minimal reproduction is still a lot of boilerplate.
I looked at updating tests, but I didn't find a spot where I could using the plugin metadata and instantiate a GraphQLModule using it.
What would you like me to do?

[CarsonF]

Bump 🙏