From 079caf6550322709aa62500681e416d1b38e233c Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 4 Nov 2020 11:59:56 +0000 Subject: [PATCH 1/6] fix: consistently set rootValue and contextValue if not overridden --- src/server.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/server.ts b/src/server.ts index 37267f01..2e0baad5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -593,14 +593,13 @@ export function createServer( ]); } - // if onsubscribe didnt return anything, inject roots - if (!maybeExecArgsOrErrors) { + // if `onSubscribe` didnt specify a rootValue, inject one + if (!('rootValue' in execArgs)) { 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) { + // if `onSubscribe` didn't specify a context, inject one + if (!('contextValue' in execArgs)) { execArgs.contextValue = typeof context === 'function' ? context(ctx, message, execArgs) From 593c86adee176b512ee303575cb5b088c7018e64 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 4 Nov 2020 13:27:05 +0000 Subject: [PATCH 2/6] Fix test --- src/tests/server.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/server.ts b/src/tests/server.ts index cdc55f72..f19fb31e 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -790,6 +790,7 @@ describe('Subscribe', () => { schema, operationName: 'Nope', document: parse(`query Nope { getValue }`), + rootValue: undefined, }; const { url } = await startTServer({ schema: undefined, From 030ca6c9c14fa4bd618abaadf0807643cba29784 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 4 Nov 2020 14:08:39 +0000 Subject: [PATCH 3/6] Tweak test again --- src/tests/server.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/server.ts b/src/tests/server.ts index f19fb31e..4fc803cd 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -790,7 +790,7 @@ describe('Subscribe', () => { schema, operationName: 'Nope', document: parse(`query Nope { getValue }`), - rootValue: undefined, + rootValue: null, }; const { url } = await startTServer({ schema: undefined, @@ -799,7 +799,7 @@ describe('Subscribe', () => { }, execute: (args) => { expect(args.schema).toBe(nopeArgs.schema); // schema from nopeArgs - expect(args.rootValue).toBeUndefined(); // nopeArgs didnt provide any root value + expect(args.rootValue).toBeNull(); // nopeArgs provided rootValue: null, so don't overwrite expect(args.operationName).toBe('Nope'); expect(args.variableValues).toBeUndefined(); // nopeArgs didnt provide variables return execute(args); From 99f32497a30553babeaa7fe970bee93582d7608b Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Wed, 4 Nov 2020 20:39:40 +0100 Subject: [PATCH 4/6] docs: clarify --- src/server.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/server.ts b/src/server.ts index 43e69be2..ab23aa8b 100644 --- a/src/server.ts +++ b/src/server.ts @@ -58,6 +58,7 @@ export type GraphQLExecutionContextValue = | number | string | boolean + | undefined | null; export interface ServerOptions { @@ -96,9 +97,9 @@ export interface ServerOptions { * alongside the schema. Learn more about them * here: https://graphql.org/learn/execution/#root-fields-resolvers. * - * If you return from the `onSubscribe` callback, the - * root field 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 `rootValue` field, the relevant operation root + * will be used instead. */ roots?: { [operation in OperationTypeNode]?: Record< @@ -177,10 +178,11 @@ 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. 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. + * be directly plugged in the operation execution. + * + * Omitting the fields `contextValue` or `rootValue` + * from the returned value will have the provided server + * options fill in the gaps. * * To report GraphQL errors simply return an array * of them from the callback, they will be reported From 17b7d188ba9b837175cd1801534aa7a6fe7e92b7 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Wed, 4 Nov 2020 20:43:15 +0100 Subject: [PATCH 5/6] test: use root from roots --- src/tests/server.ts | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/tests/server.ts b/src/tests/server.ts index 4fc803cd..c3ccfc65 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -364,6 +364,51 @@ it('should pass the `onSubscribe` exec args to the `context` option and use it', ); }); +it('should use the root from the `roots` option if the `onSubscribe` doesnt provide one', async (done) => { + const rootValue = {}; + const execArgs = { + // no rootValue here + schema, + document: parse(`query { getValue }`), + }; + + const { url } = await startTServer({ + roots: { + query: rootValue, + }, + onSubscribe: () => { + return execArgs; + }, + execute: (args) => { + expect(args).toBe(execArgs); // from `onSubscribe` + expect(args.rootValue).toBe(rootValue); // injected by `roots` + 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('should prefer the `onSubscribe` context value even if `context` option is set', async (done) => { const context = 'not-me'; const execArgs = { From 259dbd122029189c39361b372306f000936e385a Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Wed, 4 Nov 2020 20:44:12 +0100 Subject: [PATCH 6/6] docs: generate [skip ci] --- docs/interfaces/_server_.serveroptions.md | 15 ++++++++------- docs/modules/_server_.md | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/interfaces/_server_.serveroptions.md b/docs/interfaces/_server_.serveroptions.md index 4e3ef748..0003be31 100644 --- a/docs/interfaces/_server_.serveroptions.md +++ b/docs/interfaces/_server_.serveroptions.md @@ -204,10 +204,11 @@ 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. 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. +be directly plugged in the operation execution. + +Omitting the fields `contextValue` or `rootValue` +from the returned value will have the provided server +options fill in the gaps. To report GraphQL errors simply return an array of them from the callback, they will be reported @@ -233,9 +234,9 @@ The GraphQL root fields or resolvers to go alongside the schema. Learn more about them here: https://graphql.org/learn/execution/#root-fields-resolvers. -If you return from the `onSubscribe` callback, the -root field 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 `rootValue` field, the relevant operation root +will be used instead. ___ diff --git a/docs/modules/_server_.md b/docs/modules/_server_.md index f333a4a1..07c73d72 100644 --- a/docs/modules/_server_.md +++ b/docs/modules/_server_.md @@ -25,7 +25,7 @@ ### GraphQLExecutionContextValue -Ƭ **GraphQLExecutionContextValue**: object \| symbol \| number \| string \| boolean \| null +Ƭ **GraphQLExecutionContextValue**: object \| symbol \| number \| string \| boolean \| undefined \| null A concrete GraphQL execution context value type.