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

Lack of typescript support in subscriptions configuration section (unable to set up graphql-ws) #2376

Open
1 task done
Kasheftin opened this issue Aug 31, 2022 · 3 comments
Labels

Comments

@Kasheftin
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

I have an issue when trying to set up graphql subscriptions using graphql-ws. I need to specify custom port (for being able serving wss/https using nginx) and append context with custom loaders (they use DataLoader under the hood). However documentation does not say anything about the port.

What's more important, the typing in subscriptions configuration section is missing. That's how my app.module.ts looks like:

@Module({
  imports: [
    ...
    GraphQLModule.forRootAsync({
      driver: ApolloDriver,
      imports: [TasksModule],
      inject: [TasksService],
      useFactory: (tasksService: TasksService) => ({
        autoSchemaFile: join(process.cwd(), 'src/schema.gql'),
        context: () => createTaskLoaders(tasksService),
        subscriptions: { // no typescript support for the entire subscriptions section
          'graphql-ws': {
            anyOptionName: 'you can put anything here - it's not checked',
            onConnect: (context) => {
              context.extra.test = 'context is not typed as well'
            }
          },
          'any-other-library-does-not-matter-if-installed-or-not': true
        }
      })
    })
  ]
})

Describe the solution you'd like

It would be nice to have type safe in such a critical place as root level set up.
Also, since subscriptions-transport-ws is going to be deprecated, it would be nice to have the documentation being focused on graphql-ws usage overall.

Teachability, documentation, adoption, migration strategy

No response

What is the motivation / use case for changing the behavior?

I'm stuck. I can not set up the basic subscription hello world using graphql-ws and serve it through wss with nginx.

@Nickersoft
Copy link

+1 on the lack of typing around graphql-ws. I actually just had a conversation over in the discussion forum for graphql-ws about this exact issue before I realized it was a lack of typing on the part of this repo: enisdenjo/graphql-ws#257 (reply in thread).

@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

@bneigher
Copy link

bneigher commented Dec 2, 2022

I got it going with the following:

import { Context } from 'apollo-server-core'
import { Context as WSContext } from 'graphql-ws'
{
    ...,
    subscriptions: {
      'graphql-ws': {
        onConnect: (context: WSContext) => {
          return setupContext(context)
        },
        onDisconnect: (context: WSContext) => {
          //
        }
      }
    },
    context: async (context: Context<any>) => {
      return setupContext(context)
    }
}

Then a setupContext function which handles both ws requests and http requests... be aware the Context object is slightly different but the types should help from the imports as shown above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants