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

FieldWrapper wraps the final element instead of the overall field type #4614

Closed
richiejd opened this issue Aug 23, 2020 · 4 comments
Closed
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@richiejd
Copy link

Basic example:

  • expected: FieldWrapper<Maybe<Array<Maybe<Favorite>>>>
  • actual: Maybe<Array<Maybe<FieldWrapper<Address>>>>

Not sure whether to call this a bug or just a missing feature, but the FieldWrapper is put inside rather than outside of the Array and Maybe wrappers.

Example:
With the config:

      defaultMapper: Partial<{T}>
      fieldWrapperValue: T | Promise<T>
      wrapFieldDefinitions: true

Given the following graphql (note I'm contriving an example, but don't usually have nullables in an array...):

type User {
  addresses: [Address!]!
  orders: [Order]!
  saved: [Saved!]
  favorites: [Favorite]
  defaultAddress: Address!
  favoriteAddress: Address
}

I would expect the following ts:

export type User = {
  __typename?: "User";
  addresses: FieldWrapper<Array<Address>>;
  orders: FieldWrapper<Array<Maybe<Order>>>;
  saved?: FieldWrapper<Maybe<Array<Saved>>>;
  favorites?: FieldWrapper<Maybe<Array<Maybe<Favorite>>>>;
  defaultAddress: FieldWrapper<Address>;
  favoriteAddress?: FieldWrapper<Maybe<Address>>;
};

instead I get:

export type User = {
  __typename?: "User";
  addresses: Array<FieldWrapper<Address>>;
  orders: Array<Maybe<FieldWrapper<Address>>>;
  saved?: Maybe<Array<FieldWrapper<Address>>>;
  favorites?: Maybe<Array<Maybe<FieldWrapper<Address>>>>;
  defaultAddress: FieldWrapper<Address>;
  favoriteAddress?: Maybe<FieldWrapper<Address>>;
};

As you can see, I would expect support for FieldWrapper to wrap the whole thing so I can have Promise<T> | T

This is on version 1.17.0

@richiejd richiejd changed the title Field wrapper wraps the final element instead of the overall field type FieldWrapper wraps the final element instead of the overall field type Aug 23, 2020
@richiejd
Copy link
Author

This same issue is actually a further issue in the resolver map, e.g. I get the following errors

Type '(_: {}, __: {}, { customerId }: Context) => Promise<CustomerRecord | null>' is not assignable to type 'ResolverFn<Maybe<ResolverTypeWrapper<Partial<Customer>>>, {}, Context, {}>'.
  Type 'Promise<CustomerRecord | null>' is not assignable to type 'Partial<Customer> | Promise<Partial<Customer>> | Promise<Maybe<ResolverTypeWrapper<Partial<Customer>>>> | null'.
    Type 'Promise<CustomerRecord | null>' is not assignable to type 'Promise<Partial<Customer>>'.
      Type 'CustomerRecord | null' is not assignable to type 'Partial<Customer>'.
        Type 'null' is not assignable to type 'Partial<Customer>'.

I'm not sure exactly how it should be configured, but the Maybe being where it is causes a mismatch on the null type.
It should be ResolverTypeWrapper<Maybe<Partial<Customer>>> to fix the issue.

@hampusborgos
Copy link
Contributor

Did you figure out a way to fix this? It makes the entire FieldWrapper pointless to use for more complicated models.

ie. having an async field on a model that returns an Array<T | null> is not really an uncommon scenario. Is there really no way to make this library output code that supports async fields? :(

@dotansimha Any input on how to use the generated code, when you have models that have Promise fields?

@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

3 participants