fix(server): Consistently set rootValue and contextValue, if not …

…overridden (enisdenjo#49)

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.
 
 ___
 

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.
 

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
@@ -590,14 +592,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)

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<MessageType.ConnectionInit>({
+      type: MessageType.ConnectionInit,
+    }),
+  );
+  await client.waitForMessage(({ data }) => {
+    expect(parseMessage(data).type).toBe(MessageType.ConnectionAck);
+  });
+
+  client.ws.send(
+    stringifyMessage<MessageType.Subscribe>({
+      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 = {
@@ -790,6 +835,7 @@ describe('Subscribe', () => {
       schema,
       operationName: 'Nope',
       document: parse(`query Nope { getValue }`),
+      rootValue: null,
     };
     const { url } = await startTServer({
       schema: undefined,
@@ -798,7 +844,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);