Skip to content
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.

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

fix: Field Middlewares do not work on interfaces of interfaces #2666

Merged
merged 2 commits into from
Mar 10, 2023

Conversation

roypeled
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • [ x] 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?

The problem - If you have a class Foo, which implements a GQL interface IBar, and IBar implements GQL interface IBaz, field middlewares defined on IBar would work, but on IBaz they would get ignored.

Issue Number: N/A

What is the new behavior?

The fix creates a new private method which will recursively get the interfaces of a class, and the interfaces of the returned interfaces, and merge all the fields into an array.
Eventually, all interfaces in the inheritance chain are checked, all fields are collected, and no middleware is ignored.

Does this PR introduce a breaking change?

  • Yes
  • [n ] No

Other information

The problem - If you have a class Foo, which implements a GQL interface IBar, and IBar implements GQL interface IBaz, field middlewares defined on IBar would work, but on IBaz they would get ignored.

object-type-definition.factory.ts#generateFields will create a custom field resolver if a middleware definition exists on that field.
Before the fix, the method would get a class; if it implements a (GQL) interface it will only get its interface and merge the interface fields with the class' fields, ignoring any interfaces implemented by the class' first interface.

The fix creates a new private method which will recursively get the interfaces of a class, and the interfaces of the returned interfaces, and merge all the fields into an array.
@roypeled
Copy link
Contributor Author

Hey @kamilmysliwiec, I found this bug while extending our schemas. I discovered that field middleware for interfaces would just get ignored.
I found out that middleware are only counted for the direct interface of a class, and not on any subsequent interfaces.

I would be happy for this to be merged quickly, since this is a major pain for us at the moment.

Thanks!

@roypeled
Copy link
Contributor Author

roypeled commented Mar 7, 2023

@kamilmysliwiec any update on this?

@kamilmysliwiec
Copy link
Member

LGTM

@kamilmysliwiec kamilmysliwiec merged commit 5307fe4 into nestjs:master Mar 10, 2023
@roypeled
Copy link
Contributor Author

Thanks!

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