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

Subscription resolver args are of type any #6482

Open
Tracked by #8296 ...
puchm opened this issue Aug 14, 2021 · 1 comment
Open
Tracked by #8296 ...

Subscription resolver args are of type any #6482

puchm opened this issue Aug 14, 2021 · 1 comment
Labels
core Related to codegen core/cli stage/1-reproduction A reproduction exists

Comments

@puchm
Copy link
Contributor

puchm commented Aug 14, 2021

Describe the bug

It seems to me like arguments for subscription resolvers are all of type any. I got this after running the code generator with the Typescript Resolvers plugin:

export type SubscriptionResolver<TResult, TKey extends string, TParent = {}, TContext = {}, TArgs = {}> =
  | ((...args: any[]) => SubscriptionObject<TResult, TKey, TParent, TContext, TArgs>)
  | SubscriptionObject<TResult, TKey, TParent, TContext, TArgs>;

What is the reason for ...args: any[]?

To Reproduce
Steps to reproduce the behavior:
Run the code generator with the typescript resolvers plugin on a schema with subscriptions. I think this should result in the same issue.
Reproduction Repo: https://codesandbox.io/s/magical-pare-hl1sv?file=/schema.graphql

Check line 88 in types.ts.

  1. My GraphQL schema:
type Query {
    user(id: ID!): User!
}

type User {
    id: ID!
    username: String!
    email: String!
}

type Subscription {
    userUpdated(id: ID!): User
}
  1. My GraphQL operations:
# None
  1. My codegen.yml config file:
schema: schema.graphql
documents: document.graphql
generates:
  types.ts:
    plugins:
      - typescript
      - typescript-resolvers

Expected behavior

I would expect subscription resolvers to have typed arguments which should be exactly the same as the arguments for query and mutation resolvers.
Environment:
See reproduction repo, or check these version numbers:

"dependencies": {
    "@graphql-codegen/add": "2.0.2",
    "@graphql-codegen/cli": "1.20.1",
    "@graphql-codegen/typescript": "1.21.0",
    "@graphql-codegen/typescript-operations": "1.17.14",
    "@graphql-codegen/typescript-resolvers": "^2.0.0",
    "graphql": "15.5.0"
  }
@Narretz
Copy link

Narretz commented Jan 29, 2022

I don't think the type for SubscriptionResolver is the problem. It simply allows you to use a function to return the SubscriptionObject.

I think the problems are here:

export interface SubscriptionSubscriberObject<TResult, TKey extends string, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<{ [key in TKey]: TResult }, TParent, TContext, TArgs>;
  resolve?: SubscriptionResolveFn<TResult, { [key in TKey]: TResult }, TContext, TArgs>;
}

export interface SubscriptionResolverObject<TResult, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<any, TParent, TContext, TArgs>;
  resolve: SubscriptionResolveFn<TResult, any, TContext, TArgs>;
}

export type SubscriptionObject<TResult, TKey extends string, TParent, TContext, TArgs> =
  | SubscriptionSubscriberObject<TResult, TKey, TParent, TContext, TArgs>
  | SubscriptionResolverObject<TResult, TParent, TContext, TArgs>;

The union of SubscriptionSubscriberObject and SubscriptionResolverObject is apparently not compatible or something. I don't know what's the term for this, but Typescript apparentely gives up to match the types. You can see it starting to work (kind of), if you remove either SubscriptionResolverObject or SubscriptionSubscriberObject from the union.

However, with SubscriptionResolverObject you'll get any even if you define a custom parent type (with allowParentTypeOverride in codegen). What's also weird is that the resolve fn is mandatory, even though I've only ever seen it optional.

With SubscriptionSubscriberObject, the parent type is the result of the subscription even if you override the parent type, which is also wrong afaict. Because the resolve function provides the result in the first place.

I think there should be only one possible subscription object:

export interface SubscriptionSubscriberObject<TResult, TKey extends string, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<{ [key in TKey]: TResult }, TParent, TContext, TArgs>;
  resolve?: SubscriptionResolveFn<TResult, TParent, TContext, TArgs>;
}

But that's probably too simple.
I don't really understand why there are two types of SubscriptionObjects. Maybe the idea was that you could handle both the cases: the subscribe fn or the resolve fn produces the result. If the subscribe fn already produces the result, the resolve fn would get as the parent type. If the subscribe fn produces an intermediate value, this would be the parent type.
But for that to work you'd need to split the parent type and the "payload type" for resolve.

Playground link

If I find the time I'll check out the repo and see if any tests fail with these changes.

Maybe @kamilkisiela can also can take a look, as they've worked on this code before.

@charlypoly charlypoly added stage/1-reproduction A reproduction exists and removed bug/2-confirmed labels Mar 14, 2022
@charlypoly charlypoly added the core Related to codegen core/cli label Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to codegen core/cli stage/1-reproduction A reproduction exists
Projects
None yet
Development

No branches or pull requests

4 participants