Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(response-cache): accept serverContext as the 2nd param in enabled and session #3285

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quiet-moles-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'graphql-yoga': minor
---

Now `onParams` hook's payload has `serverContext`
5 changes: 5 additions & 0 deletions .changeset/twenty-birds-sin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-yoga/plugin-response-cache': patch
---

Now `enabled` and `session` factory functions take a second parameter `ServerContext` that includes the server specific context object. But this object is not the one provided by the user. [Learn more about the difference between the server context and the user context](https://the-guild.dev/graphql/yoga-server/docs/features/context#advanced-context-life-cycle)
2 changes: 1 addition & 1 deletion examples/bun-yoga-ws/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"bun-types": "^1.0.0",
"graphql": "^16.6.0",
"graphql-ws": "^5.14.1",
"graphql-yoga": "^4.0.5"
"graphql-yoga": "5.3.1"
},
"devDependencies": {
"@whatwg-node/fetch": "^0.9.0"
Expand Down
9 changes: 6 additions & 3 deletions packages/graphql-yoga/src/plugins/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export type Plugin<
* Use this hook with your own risk. It is still experimental and may change in the future.
* @internal
*/
onParams?: OnParamsHook;
onParams?: OnParamsHook<TServerContext>;
/**
* Use this hook with your own risk. It is still experimental and may change in the future.
* @internal
Expand Down Expand Up @@ -106,11 +106,14 @@ export interface OnRequestParseDoneEventPayload {
setRequestParserResult: (params: GraphQLParams | GraphQLParams[]) => void;
}

export type OnParamsHook = (payload: OnParamsEventPayload) => PromiseOrValue<void>;
export type OnParamsHook<TServerContext = unknown> = (
payload: OnParamsEventPayload<TServerContext>,
) => PromiseOrValue<void>;

export interface OnParamsEventPayload {
export interface OnParamsEventPayload<TServerContext = unknown> {
params: GraphQLParams;
request: Request;
serverContext?: TServerContext;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setParams: (params: GraphQLParams) => void;
setResult: (result: ExecutionResult | AsyncIterable<ExecutionResult>) => void;
fetchAPI: FetchAPI;
Expand Down
10 changes: 6 additions & 4 deletions packages/graphql-yoga/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export class YogaServer<
Plugin<TUserContext & TServerContext & YogaInitialContext, TServerContext>
>;
private onRequestParseHooks: OnRequestParseHook<TServerContext>[];
private onParamsHooks: OnParamsHook[];
private onParamsHooks: OnParamsHook<TServerContext>[];
private onResultProcessHooks: OnResultProcess[];
private maskedErrorsOpts: YogaMaskedErrorOpts | null;
private id: string;
Expand Down Expand Up @@ -446,12 +446,14 @@ export class YogaServer<
: [serverContext: TServerContext]
) {
try {
const serverContext = args[0];
let result: ExecutionResult | AsyncIterable<ExecutionResult> | undefined;

for (const onParamsHook of this.onParamsHooks) {
await onParamsHook({
params,
request,
serverContext,
setParams(newParams) {
params = newParams;
},
Expand All @@ -464,7 +466,7 @@ export class YogaServer<

if (result == null) {
const additionalContext =
args[0]?.request === request
serverContext?.request === request
? {
params,
}
Expand All @@ -473,8 +475,8 @@ export class YogaServer<
params,
};

const initialContext = args[0]
? Object.assign(batched ? Object.create(args[0]) : args[0], additionalContext)
const initialContext = serverContext
? Object.assign(batched ? Object.create(serverContext) : serverContext, additionalContext)
: additionalContext;

const enveloped = this.getEnveloped(initialContext);
Expand Down
26 changes: 14 additions & 12 deletions packages/plugins/response-cache/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ export type BuildResponseCacheKeyFunction = (
},
) => ReturnType<EnvelopBuildResponseCacheKeyFunction>;

export type UseResponseCacheParameter = Omit<
export type UseResponseCacheParameter<TServerContext> = Omit<
UseEnvelopResponseCacheParameter,
'getDocumentString' | 'session' | 'cache' | 'enabled' | 'buildResponseCacheKey'
> & {
cache?: Cache;
session: (request: Request) => PromiseOrValue<Maybe<string>>;
enabled?: (request: Request) => boolean;
session: (request: Request, serverContext?: TServerContext) => PromiseOrValue<Maybe<string>>;
enabled?: (request: Request, serverContext?: TServerContext) => boolean;
Comment on lines +28 to +29
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is serverContext optional? Can conditional optional based on the provided TServerContext type instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, the server context object passed here is TServerContext | undefined;
https://github.com/dotansimha/graphql-yoga/pull/3285/files#diff-2bb04768b60ebe5776d0a80eafa2b96026b7bae21deda57af2e04bb9e5abb4cbR445
It doesn't work otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is | undefined, serverContext: TServerContext should be fine?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the ? is the same as if it is undefined?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what scenario is the server context undefined in real-world usage?

buildResponseCacheKey?: BuildResponseCacheKeyFunction;
};

Expand Down Expand Up @@ -82,7 +82,9 @@ export interface Cache extends EnvelopCache {
>;
}

export function useResponseCache(options: UseResponseCacheParameter): Plugin {
export function useResponseCache<TServerContext extends Record<string, unknown>>(
options: UseResponseCacheParameter<TServerContext>,
): Plugin<TServerContext, TServerContext> {
const buildResponseCacheKey: BuildResponseCacheKeyFunction =
options?.buildResponseCacheKey || defaultBuildResponseCacheKey;
const cache = options.cache ?? createInMemoryCache();
Expand All @@ -94,10 +96,10 @@ export function useResponseCache(options: UseResponseCacheParameter): Plugin {
},
onPluginInit({ addPlugin }) {
addPlugin(
useEnvelopResponseCache({
useEnvelopResponseCache<YogaInitialContext & TServerContext>({
...options,
enabled({ request }) {
return enabled(request);
enabled(ctx: YogaInitialContext & TServerContext) {
return enabled(ctx.request, ctx);
},
cache,
getDocumentString: getDocumentStringForEnvelop,
Expand Down Expand Up @@ -129,8 +131,8 @@ export function useResponseCache(options: UseResponseCacheParameter): Plugin {
}),
);
},
async onRequest({ request, fetchAPI, endResponse }) {
if (enabled(request)) {
async onRequest({ request, serverContext, fetchAPI, endResponse }) {
if (enabled(request, serverContext)) {
const operationId = request.headers.get('If-None-Match');
if (operationId) {
const cachedResponse = await cache.get(operationId);
Expand Down Expand Up @@ -158,8 +160,8 @@ export function useResponseCache(options: UseResponseCacheParameter): Plugin {
}
}
},
async onParams({ params, request, setResult }) {
const sessionId = await options.session(request);
async onParams({ params, request, serverContext, setResult }) {
const sessionId = await options.session(request, serverContext);
const operationId = await buildResponseCacheKey({
documentString: params.query || '',
variableValues: params.variables,
Expand All @@ -169,7 +171,7 @@ export function useResponseCache(options: UseResponseCacheParameter): Plugin {
});
operationIdByRequest.set(request, operationId);
sessionByRequest.set(request, sessionId);
if (enabled(request)) {
if (enabled(request, serverContext)) {
const cachedResponse = await cache.get(operationId);
if (cachedResponse) {
const responseWithSymbol = {
Expand Down
51 changes: 3 additions & 48 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.