mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.34k stars 234 forks source link

GraphQL Subscriptions onConnect access context and socket #435

Open teovillanueva opened 3 years ago

teovillanueva commented 3 years ago

Hi, I’m currently trying to implement authentication for my subscriptions and i found out the onConnect method is very limited. I can't access the context to set some properties neither the socket. I can open a PR, let me know if this makes sense ;)

teovillanueva commented 3 years ago

Looking at this test,

test('subscription connection extends context with onConnect return value', async (t) => {
  t.plan(3)

  const context = {
    a: 1
  }

  const sc = new SubscriptionConnection({
    on () {},
    close () {},
    send (message) {
      t.equal(JSON.stringify({ type: 'connection_ack' }), message)
    }
  }, {
    context,
    onConnect: function () {
      return { data: true }
    }
  })

  await sc.handleConnectionInit({})
  t.is(sc.context.data, true)
  t.is(sc.context.a, 1)
})

whatever is returned in the onConnect is added to the context right?

PabloSzx commented 3 years ago

whatever is returned in the onConnect is added to the context right?

yes

teovillanueva commented 3 years ago

Cool!

SkeLLLa commented 3 years ago

I also wanted to modify onConnect behavior, since now it's just uses shallow merge which means that some nested props will be overridden.

So what I've found so far that this argument in onConnect function points to SubscriptionConnection instance which has context property. Is this behavior considered as valid? If so, then I guess it needs to be documented and types for SubscriptionConnection are missing.

Or maybe it will be good if we allow option to choose shallow/deep merge?

pasha-vuiko commented 2 years ago

any updates on this?