Skip to content

Commit

Permalink
Change hookArgs() to accept parameters in the same order as grafast()
Browse files Browse the repository at this point in the history
  • Loading branch information
benjie committed Mar 10, 2023
1 parent faaf55f commit 7a84f64
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 36 deletions.
4 changes: 3 additions & 1 deletion grafast/grafast/src/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import { isPromiseLike } from "./utils.js";
*/
export function hookArgs(
args: ExecutionArgs,
resolvedPreset: GraphileConfig.ResolvedPreset,
ctx: Partial<Grafast.RequestContext>,
resolvedPreset: GraphileConfig.ResolvedPreset = NULL_PRESET,
): ExecutionArgs | PromiseLike<ExecutionArgs> {
// TODO: assert that args haven't already been hooked

// Make context mutable
args.contextValue = Object.assign(Object.create(null), args.contextValue);

Expand Down
10 changes: 5 additions & 5 deletions grafast/grafast/src/grafastGraphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import type {
import { GraphQLError, parse, Source, validate, validateSchema } from "graphql";
import type { PromiseOrValue } from "graphql/jsutils/PromiseOrValue";

import { NULL_PRESET } from "./config.js";
import { SafeError } from "./error.js";
import { execute } from "./execute.js";
import { hookArgs } from "./index.js";
Expand Down Expand Up @@ -151,7 +150,7 @@ export function grafastGraphql(
};

if (resolvedPreset && ctx) {
const argsOrPromise = hookArgs(executionArgs, ctx, resolvedPreset);
const argsOrPromise = hookArgs(executionArgs, resolvedPreset, ctx);
if (isPromiseLike(argsOrPromise)) {
return Promise.resolve(argsOrPromise).then((hookedArgs) =>
execute(hookedArgs, resolvedPreset),
Expand All @@ -162,15 +161,16 @@ export function grafastGraphql(
}
} else {
// Execute
return execute(executionArgs, resolvedPreset ?? NULL_PRESET);
return execute(executionArgs, resolvedPreset);
}
}

export function grafastGraphqlSync(
args: GraphQLArgs,
resolvedPreset: GraphileConfig.ResolvedPreset = NULL_PRESET,
resolvedPreset?: GraphileConfig.ResolvedPreset,
ctx?: Partial<Grafast.RequestContext>,
): ExecutionResult {
const result = grafastGraphql(args, resolvedPreset);
const result = grafastGraphql(args, resolvedPreset, ctx);
if (isPromiseLike(result)) {
throw new SafeError("Grafast execution failed to complete synchronously.");
}
Expand Down
12 changes: 4 additions & 8 deletions grafast/grafserv/src/middleware/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,10 @@ export const makeGraphQLHandler = (
operationName,
};

await hookArgs(
args,
{
...request.requestContext,
http: request,
},
resolvedPreset,
);
await hookArgs(args, resolvedPreset, {
...request.requestContext,
http: request,
});

try {
const result = await grafastExecute(args, resolvedPreset);
Expand Down
20 changes: 6 additions & 14 deletions grafast/grafserv/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,23 +165,15 @@ export function makeGraphQLWSConfig(instance: GrafservBase): ServerOptions {
schema: async () => instance.getSchema(),
// PERF: we can remove the async/await and only use when context is async
execute: async (args: ExecutionArgs) => {
await hookArgs(
args,
{
ws: (args.contextValue as any)?.[$$extra],
},
instance.resolvedPreset,
);
await hookArgs(args, instance.resolvedPreset, {
ws: (args.contextValue as any)?.[$$extra],
});
return maskExecutionResult(await execute(args, resolvedPreset));
},
subscribe: async (args: ExecutionArgs) => {
await hookArgs(
args,
{
ws: (args.contextValue as any)?.[$$extra],
},
instance.resolvedPreset,
);
await hookArgs(args, instance.resolvedPreset, {
ws: (args.contextValue as any)?.[$$extra],
});
return maskExecutionResult(await subscribe(args, resolvedPreset));
},
};
Expand Down
2 changes: 1 addition & 1 deletion postgraphile/postgraphile/__tests__/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export async function runTestQuery(
variableValues,
};

await hookArgs(args, {}, resolvedPreset);
await hookArgs(args, resolvedPreset, {});

// We must override the context so that we can listen to the SQL queries.
args.contextValue = {
Expand Down
10 changes: 3 additions & 7 deletions postgraphile/website/postgraphile/migrating-from-v4/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,9 @@ async function main() {
};

// Merge in the context and anything else plugins/presets want to add
await hookArgs(
args,
{
/* optional details for your context callback(s) to use */
},
resolvedPreset,
);
await hookArgs(args, resolvedPreset, {
/* optional details for your context callback(s) to use */
});

const result = await grafast(args);

Expand Down

0 comments on commit 7a84f64

Please sign in to comment.