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: metadata storage refactor caused a bug in field resolver #2724

Merged
merged 1 commit into from Mar 20, 2023

Conversation

roypeled
Copy link
Contributor

When creating a field resolver with an input we reached a bad edge case where the fields array was fetched via reference instead of an immutable array. This caused artifacts appearing in the array and bad metadata appeared when trying to parse the schema.

Returning a new array from the metadata instead of the reference for the existing array solves this issue

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

#2360 (comment)

Issue Number: 2360

What is the new behavior?

Fields resolver works correctly.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

When creating a field resolver with an input we reached a bad edge case where the fields array was fetched via reference instead of an immutable array. This caused artifacts appearing in the array and bad metadata appeared when trying to parse the schema.

Returning a new array from the metadata instead of the reference for the existing array solves this issue
@kamilmysliwiec
Copy link
Member

lgtm

@kamilmysliwiec kamilmysliwiec merged commit 14dd68c into nestjs:master Mar 20, 2023
@jhpierce
Copy link

Thank you both for the fix. Is there any chance this could be backported to v10?

@roypeled
Copy link
Contributor Author

I don’t see a reason not to, it’s a small fix.
@kamilmysliwiec should I create a pull request?

@jhpierce
Copy link

I'm also happy to put up that PR if need be!

@jhpierce
Copy link

@kamilmysliwiec @roypeled I created a PR to backport this fix to v10. Please advise on how to proceed, thank you!

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.

None yet

3 participants