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

[typescript-resolvers] Field wrapper doesn't wrap array types #5227

Closed
nebbles opened this issue Dec 9, 2020 · 3 comments
Closed

[typescript-resolvers] Field wrapper doesn't wrap array types #5227

nebbles opened this issue Dec 9, 2020 · 3 comments
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@nebbles
Copy link
Contributor

nebbles commented Dec 9, 2020

Describe the bug
The change made for #3882 is counter-intuitive to me and seems like a bug. By my understanding, a Field definition includes whether its an array or not.

Based on this change, when adding to config:

wrapFieldDefinitions: true
fieldWrapperValue: T | Promise<T> | (() => T) | (() => Promise<T>)

Then I'd expect this wrapper to be applied to the Type of the field

// a type like this
images: Array<GqlImageData>;

// turns into
images: FieldWrapper<Array<GqlImageData>>;

However as mentioned, the above change means that Arrays cannot be given a custom wrapper. The result of using the config returns

images: Array<FieldWrapper<GqlImageData>>;

Is this a bug or a feature request? As alluded to in the example config, it would simplify code a lot if a custom field wrapper enabled resolver functions for fields (I've read #1704 - however moving to partials removes type strictness which is useful to developers in the team).

Thanks

@dotansimha dotansimha changed the title Field wrapper doesn't wrap array types [typescript-resolvers] Field wrapper doesn't wrap array types Jan 21, 2021
@ibratoev
Copy link

I hit the same issue as I use the same fieldWrapperValue. Added a few comments in #3882 about it. In my case I get the error Type '() => Promise<User[]>' is missing the following properties from type 'FieldWrapper<User>[]': pop, push, concat, join, and 27 more.

@hampusborgos
Copy link
Contributor

I have attempted to solve this in a new PR: #5753 that adds a new configuration option target to arrays (and Maybe<>) wrappers.

@dotansimha dotansimha added waiting-for-release Fixed/resolved, and waiting for the next stable release and removed need-help labels Apr 19, 2021
@dotansimha
Copy link
Owner

Fix is available in @graphql-codegen/[email protected]. Thanks @hampusborgos !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

4 participants