Skip to content

Commit

Permalink
More assertNotAsync calls (#2190)
Browse files Browse the repository at this point in the history
  • Loading branch information
@benjie
benjie authored Oct 1, 2024
2 parents b97469b + 374f846 commit f6d504e
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 59 deletions.
7 changes: 7 additions & 0 deletions .changeset/pretty-knives-crash.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"postgraphile": patch
"grafast": patch
---

Apply `assertNotAsync` and `assertNotPromise` in more places to detect plan
resolvers returning promises.
36 changes: 17 additions & 19 deletions grafast/grafast/src/engine/OperationPlan.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ import { graphqlResolver } from "../steps/graphqlResolver.js";
import { timeSource } from "../timeSource.js";
import type { Sudo } from "../utils.js";
import {
assertNotAsync,
assertNotPromise,
defaultValueToValueNode,
findVariableNamesUsed,
hasItemPlan,
Expand Down Expand Up @@ -1050,12 +1052,9 @@ export class OperationPlan {
const rawPlanResolver = objectField.extensions?.grafast?.plan;
if (rawPlanResolver) {
resolverEmulation = false;
}
if (rawPlanResolver?.constructor?.name === "AsyncFunction") {
throw new Error(
`Plans must be synchronous, but this schema has an async function at '${
objectType.name
}.${fieldName}.plan': ${rawPlanResolver.toString()}`,
assertNotAsync(
rawPlanResolver,
`${objectType.name}.${fieldName}.plan`,
);
}
const namedReturnType = getNamedType(fieldType);
Expand Down Expand Up @@ -1441,24 +1440,23 @@ export class OperationPlan {
}
} else if (isScalarType(nullableFieldType)) {
const scalarPlanResolver = nullableFieldType.extensions?.grafast?.plan;
if (scalarPlanResolver?.constructor?.name === "AsyncFunction") {
throw new Error(
`Plans must be synchronous, but this schema has an async function at '${
nullableFieldType.name
}.plan': ${scalarPlanResolver.toString()}`,
);
}
const fnDescription = `${nullableFieldType.name}.plan`;
assertNotAsync(scalarPlanResolver, fnDescription);
const $sideEffect = parentLayerPlan.latestSideEffectStep;
try {
const $leaf =
typeof scalarPlanResolver === "function"
? withGlobalLayerPlan(
parentLayerPlan,
polymorphicPaths,
? assertNotPromise(
withGlobalLayerPlan(
parentLayerPlan,
polymorphicPaths,
scalarPlanResolver,
null,
$step,
this.scalarPlanInfo,
),
scalarPlanResolver,
null,
$step,
this.scalarPlanInfo,
fnDescription,
)
: $step;

Expand Down
122 changes: 82 additions & 40 deletions grafast/grafast/src/operationPlan-input.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type { __ItemStep } from "./steps/__item.js";
import { constant, ConstantStep } from "./steps/constant.js";
import { list } from "./steps/list.js";
import { object } from "./steps/object.js";
import { assertNotAsync, assertNotPromise } from "./utils.js";

const {
getNullableType,
Expand Down Expand Up @@ -180,20 +181,34 @@ export function withFieldArgsForArguments<
if (rest.length === 0) {
// Argument
const arg = entity as GraphQLArgument;
result = arg.extensions.grafast?.inputPlan
? arg.extensions.grafast.inputPlan($parent, childFieldArgs, {
schema,
entity: arg,
})
const argInputPlan = arg.extensions.grafast?.inputPlan;
const fnDescription = `???.${field.name}(${arg.name}:).inputPlan`;
assertNotAsync(argInputPlan, fnDescription);
result = argInputPlan
? assertNotPromise(
argInputPlan($parent, childFieldArgs, {
schema,
entity: arg,
}),
argInputPlan,
fnDescription,
)
: childFieldArgs.get();
} else {
// Input field
const inputField = entity as GraphQLInputField | undefined;
result = inputField?.extensions.grafast?.inputPlan
? inputField.extensions.grafast.inputPlan(childFieldArgs, {
schema,
entity: inputField,
})
const inputFieldInputPlan = inputField?.extensions.grafast?.inputPlan;
const fnDescription = `???.${inputField?.name}.inputPlan`;
assertNotAsync(inputFieldInputPlan, fnDescription);
result = inputFieldInputPlan
? assertNotPromise(
inputFieldInputPlan(childFieldArgs, {
schema,
entity: inputField,
}),
inputFieldInputPlan,
fnDescription,
)
: childFieldArgs.get();
}
const nullableType = getNullableType(entityType);
Expand Down Expand Up @@ -289,28 +304,34 @@ export function withFieldArgsForArguments<
if (rest.length === 0) {
// Argument
const arg = entity as GraphQLArgument;
result = arg.extensions.grafast?.applyPlan
? arg.extensions.grafast.applyPlan(
$parent,
$target,
childFieldArgs,
{
const argApplyPlan = arg.extensions.grafast?.applyPlan;
const fnDescription = `???.${field.name}(${arg.name}:).applyPlan`;
assertNotAsync(argApplyPlan, fnDescription);
result = argApplyPlan
? assertNotPromise(
argApplyPlan($parent, $target, childFieldArgs, {
schema,
entity: arg,
},
}),
argApplyPlan,
fnDescription,
)
: undefined;
} else if (entity) {
// input field
const inputField = entity as GraphQLInputField;
result = inputField.extensions.grafast?.applyPlan
? inputField.extensions.grafast.applyPlan(
$target,
childFieldArgs,
{
const inputFieldApplyPlan =
inputField.extensions.grafast?.applyPlan;
const fnDescription = `???.${inputField.name}.applyPlan`;
assertNotAsync(inputFieldApplyPlan, fnDescription);
result = inputFieldApplyPlan
? assertNotPromise(
inputFieldApplyPlan($target, childFieldArgs, {
schema,
entity: inputField,
},
}),
inputFieldApplyPlan,
fnDescription,
)
: undefined;
} else {
Expand Down Expand Up @@ -380,12 +401,18 @@ export function withFieldArgsForArguments<
const typeResolver =
nullableEntityType.extensions.grafast?.inputPlan ||
defaultInputObjectTypeInputPlanResolver;
const resolvedResult = typeResolver(
getFieldArgsForPath(path, nullableEntityType, $input, "input"),
{
schema,
type: nullableEntityType,
},
const fnDescription = `${nullableEntityType}.inputPlan`;
assertNotAsync(typeResolver, fnDescription);
const resolvedResult = assertNotPromise(
typeResolver(
getFieldArgsForPath(path, nullableEntityType, $input, "input"),
{
schema,
type: nullableEntityType,
},
),
typeResolver,
fnDescription,
);
if (mode === "apply") {
// NOTE: if mode === 'input' then `.get()` won't need a
Expand All @@ -403,11 +430,17 @@ export function withFieldArgsForArguments<
} else if (isScalarType(nullableEntityType)) {
const scalarResolver =
nullableEntityType.extensions.grafast?.inputPlan;
const fnDescription = `${nullableEntityType}.inputPlan`;
assertNotAsync(scalarResolver, fnDescription);
if (scalarResolver !== undefined) {
return scalarResolver($input, {
schema,
type: nullableEntityType,
});
return assertNotPromise(
scalarResolver($input, {
schema,
type: nullableEntityType,
}),
scalarResolver,
fnDescription,
);
} else {
return $input;
}
Expand Down Expand Up @@ -480,13 +513,21 @@ export function withFieldArgsForArguments<
const enumValue = nullableEntityType
.getValues()
.find((v) => v.value === value);
const enumResolver = enumValue?.extensions.grafast?.applyPlan;
if (enumResolver !== undefined) {
const $target =
typeof targetStepOrCallback === "function"
? targetStepOrCallback()
: targetStepOrCallback;
enumResolver($target);
if (enumValue) {
const enumResolver = enumValue.extensions.grafast?.applyPlan;
const fnDescription = `${nullableEntityType.name}.${enumValue.name}.applyPlan`;
assertNotAsync(enumResolver, fnDescription);
if (enumResolver !== undefined) {
const $target =
typeof targetStepOrCallback === "function"
? targetStepOrCallback()
: targetStepOrCallback;
assertNotPromise(
enumResolver($target),
enumResolver,
fnDescription,
);
}
}
} else {
const never: never = nullableEntityType;
Expand Down Expand Up @@ -533,6 +574,7 @@ export function withFieldArgsForArguments<
}

const result = callback(fieldArgs);
assertNotPromise(result, callback, operationPlan.loc?.join(">") ?? "???");

if (!explicitlyApplied) {
processAfter(fieldArgs, [], result, args, applyAfterMode);
Expand Down
16 changes: 16 additions & 0 deletions grafast/grafast/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,13 +1084,29 @@ export function stepsAreInSamePhase(
// ENHANCE: implement this!
export const canonicalJSONStringify = (o: object) => JSON.stringify(o);

// PERF: only do this if isDev; otherwise replace with NOOP?
export function assertNotAsync(fn: any, name: string): void {
if (fn?.constructor?.name === "AsyncFunction") {
throw new Error(
`Plans must be synchronous, but this schema has an async function at '${name}': ${fn.toString()}`,
);
}
}

// PERF: only do this if isDev; otherwise replace with NOOP?
export function assertNotPromise<TVal>(
value: TVal,
fn: any,
name: string,
): TVal {
if (isPromiseLike(value)) {
throw new Error(
`Plans must be synchronous, but this schema has an function at '${name}' that returned a promise-like object: ${fn.toString()}`,
);
}
return value;
}

export function hasItemPlan(
step: ExecutableStep & {
itemPlan?: ($item: ExecutableStep) => ExecutableStep;
Expand Down

0 comments on commit f6d504e

Please sign in to comment.