From 02c4dbd05c3da1894c4f05c7a71a5855ae1a622d Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 14:10:44 +0100 Subject: [PATCH 01/12] feat: context with func signature --- src/server.ts | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/server.ts b/src/server.ts index 620b0382..37741e45 100644 --- a/src/server.ts +++ b/src/server.ts @@ -58,11 +58,18 @@ export interface ServerOptions { * important contextual information like the currently * logged in user, or access to a database. * - * If you return from the `onSubscribe` callback, this - * context value will NOT be injected. You should add it - * in the returned `ExecutionArgs` from the callback. + * If you return from `onSubscribe`, and the returned value is + * missing the `contextValue` field, this context will be used + * instead. The returned value will be passed through as the + * execution arguments, if you use the function signature. */ - context?: unknown; + context?: + | unknown + | (( + ctx: Context, + message: SubscribeMessage, + args: ExecutionArgs, + ) => unknown); /** * The GraphQL root fields or resolvers to go * alongside the schema. Learn more about them @@ -538,7 +545,6 @@ export function createServer( const { operationName, query, variables } = message.payload; const document = typeof query === 'string' ? parse(query) : query; execArgs = { - contextValue: context, schema, operationName, document, @@ -564,6 +570,15 @@ export function createServer( ]); } + // inject the context, if provided, before the operation. + // but, only if the `onSubscribe` didnt provide one already + if (context && !execArgs.contextValue) { + execArgs.contextValue = + typeof context === 'function' + ? context(ctx, message, execArgs) + : context; + } + // if onsubscribe didnt return anything, inject roots if (!maybeExecArgsOrErrors) { execArgs.rootValue = roots?.[operationAST.operation]; From 769215119fe72c7dc87811b456ca00cde5495bbc Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 14:48:34 +0100 Subject: [PATCH 02/12] style: type safety with `GraphQLExecutionContextValue` --- src/server.ts | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/server.ts b/src/server.ts index 37741e45..7298ca6f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -43,6 +43,24 @@ export type OperationResult = | AsyncIterableIterator | ExecutionResult; +/** + * A concrete GraphQL execution context value type. + * + * Mainly used because TypeScript collapes unions + * with `any` or `unknown` to `any` or `unknown`. So, + * we use a custom type to allow definitions such as + * the `context` server option. + */ +export type GraphQLExecutionContextValue = + // eslint-disable-next-line @typescript-eslint/ban-types + | object // you can literally pass "any" JS object as the context value + | symbol + | number + | string + | boolean + | null + | undefined; + export interface ServerOptions { /** * The GraphQL schema on which the operations @@ -64,12 +82,12 @@ export interface ServerOptions { * execution arguments, if you use the function signature. */ context?: - | unknown + | GraphQLExecutionContextValue | (( ctx: Context, message: SubscribeMessage, args: ExecutionArgs, - ) => unknown); + ) => GraphQLExecutionContextValue); /** * The GraphQL root fields or resolvers to go * alongside the schema. Learn more about them From b27375e2176c992d6c98b199440d91bf63b16462 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 14:50:53 +0100 Subject: [PATCH 03/12] test: use context option --- src/tests/server.ts | 50 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/tests/server.ts b/src/tests/server.ts index 3b3af04f..a22e2035 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -318,6 +318,56 @@ it('should pass in the context value from the config', async () => { expect(subscribeFn.mock.calls[0][0].contextValue).toBe(context); }); +it('should pass the `onSubscribe` exec args to the `context` option and use it', async (done) => { + const context = {}; + const execArgs = { + // no context here + schema, + document: parse(`query { getValue }`), + }; + + const { url } = await startTServer({ + onSubscribe: () => { + return execArgs; + }, + context: (_ctx, _msg, args) => { + expect(args).toBe(args); // from `onSubscribe` + return context; // will be injected + }, + execute: (args) => { + expect(args).toBe(execArgs); // from `onSubscribe` + expect(args.contextValue).toBe(context); // injected by `context` + done(); + return execute(args); + }, + subscribe, + }); + + const client = await createTClient(url); + client.ws.send( + stringifyMessage({ + type: MessageType.ConnectionInit, + }), + ); + await client.waitForMessage(({ data }) => { + expect(parseMessage(data).type).toBe(MessageType.ConnectionAck); + }); + + client.ws.send( + stringifyMessage({ + id: '1', + type: MessageType.Subscribe, + payload: { + query: `{ getValue }`, + }, + }), + ); +}); + +it.todo( + 'should prefer the `onSubscribe` context value even if `context` option is set', +); + describe('Connect', () => { it('should refuse connection and close socket if returning `false`', async () => { const { url } = await startTServer({ From f3ef075b49f599e6380171ec5d6467526724a9cb Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 14:57:47 +0100 Subject: [PATCH 04/12] docs: slim down custom args recipe --- README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4210fa2e..ca3f806e 100644 --- a/README.md +++ b/README.md @@ -515,12 +515,12 @@ createServer(
-Server usage with custom dynamic GraphQL arguments and validation +Server usage with custom execution arguments and validation ```typescript import { parse, validate, execute, subscribe } from 'graphql'; import { createServer } from 'graphql-ws'; -import { schema, getDynamicContext, myValidationRules } from 'my-graphql'; +import { schema, myValidationRules } from 'my-graphql'; createServer( { @@ -529,7 +529,6 @@ createServer( onSubscribe: (ctx, msg) => { const args = { schema, - contextValue: getDynamicContext(ctx, msg), operationName: msg.payload.operationName, document: parse(msg.payload.query), variableValues: msg.payload.variables, From 39885bde6ff83e37f45412bb3483b9a3d1a604b3 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 14:59:58 +0100 Subject: [PATCH 05/12] docs: custom graphql context value recipe --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ca3f806e..589252c9 100644 --- a/README.md +++ b/README.md @@ -490,16 +490,18 @@ server.listen(443);
-Server usage with custom static GraphQL arguments +Server usage with custom GraphQL context value ```typescript import { validate, execute, subscribe } from 'graphql'; import { createServer } from 'graphql-ws'; -import { schema, roots, getStaticContext } from 'my-graphql'; +import { schema, roots, getDynamicContext } from 'my-graphql'; createServer( { - context: getStaticContext(), + context: (ctx, msg, args) => { + return getDynamicContext(ctx, msg, args); + }, // or static context by supplying the value direcly schema, roots, execute, From 932cdaa72a9b5ec8633c3e208155b96d15850f37 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:01:14 +0100 Subject: [PATCH 06/12] docs: unnecessary "graphql" [skip ci] --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 589252c9..fee891c6 100644 --- a/README.md +++ b/README.md @@ -490,7 +490,7 @@ server.listen(443);
-Server usage with custom GraphQL context value +Server usage with custom context value ```typescript import { validate, execute, subscribe } from 'graphql'; From e5a56854f041e6f7f72814dc903bbd885bfbacd6 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:06:04 +0100 Subject: [PATCH 07/12] test: prefer `onSubscribe` even with `context` option --- src/tests/server.ts | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/src/tests/server.ts b/src/tests/server.ts index a22e2035..32bb2e86 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -364,9 +364,48 @@ it('should pass the `onSubscribe` exec args to the `context` option and use it', ); }); -it.todo( - 'should prefer the `onSubscribe` context value even if `context` option is set', -); +it('should prefer the `onSubscribe` context value even if `context` option is set', async (done) => { + const context = 'not-me'; + const execArgs = { + contextValue: 'me-me', // my custom context + schema, + document: parse(`query { getValue }`), + }; + + const { url } = await startTServer({ + onSubscribe: () => { + return execArgs; + }, + context, // should be ignored because there is one in `execArgs` + execute: (args) => { + expect(args).toBe(execArgs); // from `onSubscribe` + expect(args.contextValue).not.toBe(context); // from `onSubscribe` + done(); + return execute(args); + }, + subscribe, + }); + + const client = await createTClient(url); + client.ws.send( + stringifyMessage({ + type: MessageType.ConnectionInit, + }), + ); + await client.waitForMessage(({ data }) => { + expect(parseMessage(data).type).toBe(MessageType.ConnectionAck); + }); + + client.ws.send( + stringifyMessage({ + id: '1', + type: MessageType.Subscribe, + payload: { + query: `{ getValue }`, + }, + }), + ); +}); describe('Connect', () => { it('should refuse connection and close socket if returning `false`', async () => { From df9d83c24a34ea7e3132638ce7b8d82eb566aadb Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:10:09 +0100 Subject: [PATCH 08/12] docs: onSubscribe docs --- src/server.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/server.ts b/src/server.ts index 7298ca6f..7ce7cce3 100644 --- a/src/server.ts +++ b/src/server.ts @@ -174,7 +174,10 @@ export interface ServerOptions { * it will be used instead of trying to build one * internally. In this case, you are responsible * for providing a ready set of arguments which will - * be directly plugged in the operation execution. + * be directly plugged in the operation execution. Beware, + * the `context` server option is an exception. Only if you + * dont provide a context alongside the returned value + * here, the `context` server option will be used instead. * * To report GraphQL errors simply return an array * of them from the callback, they will be reported From 05668a006adec83e3003cc532b839f0901ab4a2f Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:10:37 +0100 Subject: [PATCH 09/12] docs: generate [skip ci] --- docs/interfaces/_server_.serveroptions.md | 14 +++++++++----- docs/modules/_server_.md | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/docs/interfaces/_server_.serveroptions.md b/docs/interfaces/_server_.serveroptions.md index 650cf928..92b9e6ab 100644 --- a/docs/interfaces/_server_.serveroptions.md +++ b/docs/interfaces/_server_.serveroptions.md @@ -48,15 +48,16 @@ ___ ### context -• `Optional` **context**: unknown +• `Optional` **context**: [GraphQLExecutionContextValue](../modules/_server_.md#graphqlexecutioncontextvalue) \| (ctx: [Context](_server_.context.md), message: [SubscribeMessage](_message_.subscribemessage.md), args: ExecutionArgs) => [GraphQLExecutionContextValue](../modules/_server_.md#graphqlexecutioncontextvalue) A value which is provided to every resolver and holds important contextual information like the currently logged in user, or access to a database. -If you return from the `onSubscribe` callback, this -context value will NOT be injected. You should add it -in the returned `ExecutionArgs` from the callback. +If you return from `onSubscribe`, and the returned value is +missing the `contextValue` field, this context will be used +instead. The returned value will be passed through as the +execution arguments, if you use the function signature. ___ @@ -199,7 +200,10 @@ If you return `ExecutionArgs` from the callback, it will be used instead of trying to build one internally. In this case, you are responsible for providing a ready set of arguments which will -be directly plugged in the operation execution. +be directly plugged in the operation execution. Beware, +the `context` server option is an exception. Only if you +dont provide a context alongside the returned value +here, the `context` server option will be used instead. To report GraphQL errors simply return an array of them from the callback, they will be reported diff --git a/docs/modules/_server_.md b/docs/modules/_server_.md index 68e9b6d6..99bd079a 100644 --- a/docs/modules/_server_.md +++ b/docs/modules/_server_.md @@ -14,6 +14,7 @@ ### Type aliases +* [GraphQLExecutionContextValue](_server_.md#graphqlexecutioncontextvalue) * [OperationResult](_server_.md#operationresult) ### Functions @@ -22,6 +23,19 @@ ## Type aliases +### GraphQLExecutionContextValue + +Ƭ **GraphQLExecutionContextValue**: object \| symbol \| number \| string \| boolean \| null \| undefined + +A concrete GraphQL execution context value type. + +Mainly used because TypeScript collapes unions +with `any` or `unknown` to `any` or `unknown`. So, +we use a custom type to allow definitions such as +the `context` server option. + +___ + ### OperationResult Ƭ **OperationResult**: Promise\ \| ExecutionResult> \| AsyncIterableIterator\ \| ExecutionResult From ae744d24a17edaab6012da3c7cc5c5629613b6e3 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:15:23 +0100 Subject: [PATCH 10/12] docs: a bit more explanation [skip ci] --- docs/interfaces/_server_.serveroptions.md | 8 ++++++-- src/server.ts | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/interfaces/_server_.serveroptions.md b/docs/interfaces/_server_.serveroptions.md index 92b9e6ab..4e3ef748 100644 --- a/docs/interfaces/_server_.serveroptions.md +++ b/docs/interfaces/_server_.serveroptions.md @@ -56,8 +56,12 @@ logged in user, or access to a database. If you return from `onSubscribe`, and the returned value is missing the `contextValue` field, this context will be used -instead. The returned value will be passed through as the -execution arguments, if you use the function signature. +instead. + +If you use the function signature, the final execution arguments +will be passed in (also the returned value from `onSubscribe`). +Since the context is injected on every subscribe, the `SubscribeMessage` +with the regular `Context` will be passed in through the arguments too. ___ diff --git a/src/server.ts b/src/server.ts index 7ce7cce3..5e54925e 100644 --- a/src/server.ts +++ b/src/server.ts @@ -78,8 +78,12 @@ export interface ServerOptions { * * If you return from `onSubscribe`, and the returned value is * missing the `contextValue` field, this context will be used - * instead. The returned value will be passed through as the - * execution arguments, if you use the function signature. + * instead. + * + * If you use the function signature, the final execution arguments + * will be passed in (also the returned value from `onSubscribe`). + * Since the context is injected on every subscribe, the `SubscribeMessage` + * with the regular `Context` will be passed in through the arguments too. */ context?: | GraphQLExecutionContextValue From c888ddb7dd8e6b8b53382366e06103fd788d3718 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:21:43 +0100 Subject: [PATCH 11/12] fix: allow non-undefined, nullish, context values --- src/server.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/server.ts b/src/server.ts index 5e54925e..72306f57 100644 --- a/src/server.ts +++ b/src/server.ts @@ -58,8 +58,7 @@ export type GraphQLExecutionContextValue = | number | string | boolean - | null - | undefined; + | null; export interface ServerOptions { /** @@ -597,7 +596,7 @@ export function createServer( // inject the context, if provided, before the operation. // but, only if the `onSubscribe` didnt provide one already - if (context && !execArgs.contextValue) { + if (context !== undefined && !execArgs.contextValue) { execArgs.contextValue = typeof context === 'function' ? context(ctx, message, execArgs) From 37e8e16980de652a18d7d17979a8acd5ae66cb1e Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Tue, 3 Nov 2020 15:23:28 +0100 Subject: [PATCH 12/12] fix: inject roots first --- src/server.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server.ts b/src/server.ts index 72306f57..1405d163 100644 --- a/src/server.ts +++ b/src/server.ts @@ -594,6 +594,11 @@ export function createServer( ]); } + // if onsubscribe didnt return anything, inject roots + if (!maybeExecArgsOrErrors) { + execArgs.rootValue = roots?.[operationAST.operation]; + } + // inject the context, if provided, before the operation. // but, only if the `onSubscribe` didnt provide one already if (context !== undefined && !execArgs.contextValue) { @@ -603,11 +608,6 @@ export function createServer( : context; } - // if onsubscribe didnt return anything, inject roots - if (!maybeExecArgsOrErrors) { - execArgs.rootValue = roots?.[operationAST.operation]; - } - // the execution arguments have been prepared // perform the operation and act accordingly let operationResult;