Broken persisted queries support

[silvercrown07]

Hi @enisdenjo, first of all, thank you for maintaining such an awesome project!

I would like to report an issue related to persisted queries support.

As described in the recipes:

// 🛸 server
 
import { parse, ExecutionArgs } from 'graphql';
import { createHandler } from 'graphql-sse';
import { schema } from './my-graphql';
 
// a unique GraphQL execution ID used for representing
// a query in the persisted queries store. when subscribing
// you should use the `SubscriptionPayload.query` to transmit the id
type QueryID = string;
 
const queriesStore: Record<QueryID, ExecutionArgs> = {
  iWantTheGreetings: {
    schema, // you may even provide different schemas in the queries store
    document: parse('subscription Greetings { greetings }'),
  },
};
 
export const handler = createHandler({
  onSubscribe: (_req, params) => {
    const persistedQuery =
      queriesStore[String(params.extensions?.persistedQuery)];
    if (persistedQuery) {
      return {
        ...persistedQuery,
        variableValues: params.variables, // use the variables from the client
        contextValue: undefined,
      };
    }
 
    // for extra security only allow the queries from the store
    return [null, { status: 404, statusText: 'Not Found' }];
  },
});

onSubscribe would return a ExecutionArgs result, leading graphql to execute the expected resolver.

However in current implementation, the returned result from above example is treated as ExecutionResult, hence the resolver is not triggered.

graphql-sse/src/handler.ts

Lines 587 to 600 in d4890f1

	else if (
	isExecutionResult(onSubscribeResult) ||
	isAsyncIterable(onSubscribeResult)
	)
	return {
	// even if the result is already available, use
	// context because onNext and onComplete needs it
	ctx: (typeof context === 'function'
	? await context(req, params)
	: context) as Context,
	perform() {
	return onSubscribeResult;
	},
	};

graphql-sse/src/handler.ts

Lines 1107 to 1110 in d4890f1

	function isExecutionResult(val: unknown): val is ExecutionResult {
	// TODO: comprehensive check
	return isObject(val);
	}

Please let me know if I missed anything, and if more details are needed, thank you!

[enisdenjo]

Hey there, thanks for reporting! I think I know where the problem is, fix coming soon...

[enisdenjo]

🎉 This issue has been resolved in version 2.1.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀