Skip to content

[GraphQL] Fix FieldsBuilder not always fully unwrapping a type before deciding if a resolver is needed.#4251

Merged
alanpoulain merged 3 commits into
api-platform:2.6from
DavidBennettUK:graphql-fieldsbuilder-wrapped-type
Apr 27, 2021
Merged

[GraphQL] Fix FieldsBuilder not always fully unwrapping a type before deciding if a resolver is needed.#4251
alanpoulain merged 3 commits into
api-platform:2.6from
DavidBennettUK:graphql-fieldsbuilder-wrapped-type

Conversation

@DavidBennettUK

Copy link
Copy Markdown
Contributor
Q A
Branch? main
Bug fix? yes
New feature? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

This fixes FieldsBuilder not using the recursive version of getWrappedType. Without this, if the type is wrapped in both NonNull and then ListOfType, then ListOfType is returned instead of the actual type of the list items - which I think was the original intention of this code, as it's trying to decide if the base type needs a special resolver. For example, a non-null list of non-null strings ([String!]!) will now correctly return String as the wrapped type with this fix. This means that simple arrays such as this, which are already deserialized by the Symfony deserializer and therefore do not need to use a special resolver, can now work.

@alanpoulain

Copy link
Copy Markdown
Member

Hello David, thanks for the PR.
Could you:

  • target 2.6 since it's a bug fix,
  • add a unit test?

@DavidBennettUK DavidBennettUK force-pushed the graphql-fieldsbuilder-wrapped-type branch from 79d068f to 00a17ec Compare April 27, 2021 12:01
@DavidBennettUK DavidBennettUK changed the base branch from main to 2.6 April 27, 2021 12:02
@DavidBennettUK

Copy link
Copy Markdown
Contributor Author

@alanpoulain Thanks, all done 👍

@alanpoulain alanpoulain force-pushed the graphql-fieldsbuilder-wrapped-type branch from 00a17ec to 8ef71c2 Compare April 27, 2021 13:22
@alanpoulain alanpoulain merged commit 1be4676 into api-platform:2.6 Apr 27, 2021
@alanpoulain

Copy link
Copy Markdown
Member

Thank you @DavidBennettUK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants