From f9b80ae878b0bd7fd93ea38c2f62c6056a08d849 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Fri, 13 Dec 2024 13:03:13 -0800 Subject: [PATCH 1/3] [WIP] Sketch of derived context Summary: A sketch of derived contexts as described in https://github.com/captbaritone/grats/issues/159 Not sure this is how the implemenetaiton should work. Was just focusing on getting things working end to end. If we go this way, I'd want to focus a bit more on internal architecture as well as error handling. Test Plan: ghstack-source-id: 0513dda4e4937a4b1ca9b09e1dd17e68557cd2e1 Pull Request resolved: https://github.com/captbaritone/grats/pull/161 --- src/Extractor.ts | 45 +++++++++- src/TypeContext.ts | 75 +++++++++++----- src/codegen/TSAstBuilder.ts | 15 ++++ src/codegen/resolverCodegen.ts | 26 ++++++ src/lib.ts | 11 ++- src/metadata.ts | 11 +++ src/resolverSignature.ts | 9 ++ .../derived_context/derivedContextChain.ts | 46 ++++++++++ .../derivedContextChain.ts.expected | 88 +++++++++++++++++++ .../derivedContextUsedMultipleTimes.ts | 26 ++++++ ...erivedContextUsedMultipleTimes.ts.expected | 68 ++++++++++++++ ...multipleDerivedContextsSameType.invalid.ts | 26 ++++++ ...erivedContextsSameType.invalid.ts.expected | 50 +++++++++++ .../derived_context/simpleDerivedContext.ts | 21 +++++ .../simpleDerivedContext.ts.expected | 55 ++++++++++++ ...mpleDerivedContextUndefinedType.invalid.ts | 21 +++++ ...edContextUndefinedType.invalid.ts.expected | 32 +++++++ src/transforms/makeResolverSignature.ts | 57 +++++++----- src/transforms/resolveResolverParams.ts | 39 +++++++- 19 files changed, 677 insertions(+), 44 deletions(-) create mode 100644 src/tests/fixtures/derived_context/derivedContextChain.ts create mode 100644 src/tests/fixtures/derived_context/derivedContextChain.ts.expected create mode 100644 src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts create mode 100644 src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts.expected create mode 100644 src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts create mode 100644 src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContext.ts create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContext.ts.expected create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts.expected diff --git a/src/Extractor.ts b/src/Extractor.ts index 8e5ada8b..27ec539a 100644 --- a/src/Extractor.ts +++ b/src/Extractor.ts @@ -86,6 +86,7 @@ export type ExtractionSnapshot = { readonly definitions: DefinitionNode[]; readonly unresolvedNames: Map; readonly nameDefinitions: Map; + readonly implicitNameDefinitions: Map; readonly typesWithTypename: Set; readonly interfaceDeclarations: Array; }; @@ -117,6 +118,8 @@ class Extractor { // Snapshot data unresolvedNames: Map = new Map(); nameDefinitions: Map = new Map(); + implicitNameDefinitions: Map = + new Map(); typesWithTypename: Set = new Set(); interfaceDeclarations: Array = []; @@ -136,6 +139,7 @@ class Extractor { name: NameNode, kind: NameDefinition["kind"], ): void { + // @ts-ignore FIXME this.nameDefinitions.set(node, { name, kind }); } @@ -188,8 +192,12 @@ class Extractor { if (!ts.isDeclarationStatement(node)) { this.report(tag, E.contextTagOnNonDeclaration()); } else { - const name = this.gql.name(tag, "CONTEXT_DUMMY_NAME"); - this.recordTypeName(node, name, "CONTEXT"); + if (ts.isFunctionDeclaration(node)) { + this.recordDerivedContext(node, tag); + } else { + const name = this.gql.name(tag, "CONTEXT_DUMMY_NAME"); + this.recordTypeName(node, name, "CONTEXT"); + } } break; } @@ -270,6 +278,7 @@ class Extractor { definitions: this.definitions, unresolvedNames: this.unresolvedNames, nameDefinitions: this.nameDefinitions, + implicitNameDefinitions: this.implicitNameDefinitions, typesWithTypename: this.typesWithTypename, interfaceDeclarations: this.interfaceDeclarations, }); @@ -329,6 +338,38 @@ class Extractor { } } } + recordDerivedContext(node: ts.FunctionDeclaration, tag: ts.JSDocTag) { + const returnType = node.type; + if (returnType == null) { + throw new Error("Function declaration must have a return type"); + } + if (!ts.isTypeReferenceNode(returnType)) { + throw new Error("Function declaration must return an explicit type"); + } + + const funcName = this.namedFunctionExportName(node); + + if (!ts.isSourceFile(node.parent)) { + return this.report(node, E.functionFieldNotTopLevel()); + } + + const tsModulePath = relativePath(node.getSourceFile().fileName); + + const paramResults = this.resolverParams(node.parameters); + if (paramResults == null) return null; + + const name = this.gql.name(tag, "CONTEXT_DUMMY_NAME"); + this.implicitNameDefinitions.set( + { + kind: "DERIVED_CONTEXT", + name, + path: tsModulePath, + exportName: funcName?.text ?? null, + args: paramResults.resolverParams, + }, + returnType, + ); + } extractType(node: ts.Node, tag: ts.JSDocTag) { if (ts.isClassDeclaration(node)) { diff --git a/src/TypeContext.ts b/src/TypeContext.ts index 548d8473..9647f56d 100644 --- a/src/TypeContext.ts +++ b/src/TypeContext.ts @@ -11,26 +11,40 @@ import { DiagnosticResult, tsErr, gqlRelated, + DiagnosticsResult, + FixableDiagnosticWithLocation, + tsRelated, } from "./utils/DiagnosticError"; import { err, ok } from "./utils/Result"; import * as E from "./Errors"; import { ExtractionSnapshot } from "./Extractor"; +import { ResolverArgument } from "./resolverSignature"; export const UNRESOLVED_REFERENCE_NAME = `__UNRESOLVED_REFERENCE__`; -export type NameDefinition = { +export type DerivedResolverDefinition = { name: NameNode; - kind: - | "TYPE" - | "INTERFACE" - | "UNION" - | "SCALAR" - | "INPUT_OBJECT" - | "ENUM" - | "CONTEXT" - | "INFO"; + path: string; + exportName: string | null; + args: ResolverArgument[]; + kind: "DERIVED_CONTEXT"; }; +export type NameDefinition = + | { + name: NameNode; + kind: + | "TYPE" + | "INTERFACE" + | "UNION" + | "SCALAR" + | "INPUT_OBJECT" + | "ENUM" + | "CONTEXT" + | "INFO"; + } + | DerivedResolverDefinition; + type TsIdentifier = number; /** @@ -55,15 +69,40 @@ export class TypeContext { static fromSnapshot( checker: ts.TypeChecker, snapshot: ExtractionSnapshot, - ): TypeContext { + ): DiagnosticsResult { + const errors: FixableDiagnosticWithLocation[] = []; const self = new TypeContext(checker); for (const [node, typeName] of snapshot.unresolvedNames) { self._markUnresolvedType(node, typeName); } for (const [node, definition] of snapshot.nameDefinitions) { - self._recordTypeName(node, definition.name, definition.kind); + self._recordTypeName(node, definition); + } + for (const [definition, reference] of snapshot.implicitNameDefinitions) { + const declaration = self.maybeTsDeclarationForTsName(reference.typeName); + if (declaration == null) { + errors.push(tsErr(reference.typeName, E.unresolvedTypeReference())); + continue; + } + const existing = self._declarationToName.get(declaration); + if (existing != null) { + errors.push( + // TODO: Better error messages here + tsErr(declaration, "Duplicate derived contexts for given type", [ + tsRelated(reference, "One was defined here"), + gqlRelated(existing.name, "Other here"), + ]), + ); + continue; + } + + self._recordTypeName(declaration, definition); + } + + if (errors.length > 0) { + return err(errors); } - return self; + return ok(self); } constructor(checker: ts.TypeChecker) { @@ -72,13 +111,9 @@ export class TypeContext { // Record that a GraphQL construct of type `kind` with the name `name` is // declared at `node`. - private _recordTypeName( - node: ts.Declaration, - name: NameNode, - kind: NameDefinition["kind"], - ) { - this._idToDeclaration.set(name.tsIdentifier, node); - this._declarationToName.set(node, { name, kind }); + private _recordTypeName(node: ts.Declaration, definition: NameDefinition) { + this._idToDeclaration.set(definition.name.tsIdentifier, node); + this._declarationToName.set(node, definition); } // Record that a type references `node` diff --git a/src/codegen/TSAstBuilder.ts b/src/codegen/TSAstBuilder.ts index c82561b3..df4dde64 100644 --- a/src/codegen/TSAstBuilder.ts +++ b/src/codegen/TSAstBuilder.ts @@ -9,6 +9,7 @@ const F = ts.factory; * A helper class to build up a TypeScript document AST. */ export default class TSAstBuilder { + _globalNames: Map = new Map(); _imports: ts.Statement[] = []; imports: Map = new Map(); _helpers: ts.Statement[] = []; @@ -209,7 +210,21 @@ export default class TSAstBuilder { sourceFile, ); } + + // Given a desired name in the module scope, return a name that is unique. If + // the name is already taken, a suffix will be added to the name to make it + // unique. + // + // NOTE: This is not truly unique, as it only checks the names that have been + // generated through this method. In the future we could add more robust + // scope/name tracking. + getUniqueName(name: string): string { + const count = this._globalNames.get(name) ?? 0; + this._globalNames.set(name, count + 1); + return count === 0 ? name : `${name}_${count}`; + } } + function replaceExt(filePath: string, newSuffix: string): string { const ext = path.extname(filePath); return filePath.slice(0, -ext.length) + newSuffix; diff --git a/src/codegen/resolverCodegen.ts b/src/codegen/resolverCodegen.ts index 7b3ea745..499f8434 100644 --- a/src/codegen/resolverCodegen.ts +++ b/src/codegen/resolverCodegen.ts @@ -20,6 +20,7 @@ const F = ts.factory; */ export default class ResolverCodegen { _helpers: Set = new Set(); + _derivedContextNames: Map = new Map(); constructor(public ts: TSAstBuilder, public _resolvers: Metadata) {} resolveMethod( fieldName: string, @@ -178,11 +179,36 @@ export default class ResolverCodegen { F.createIdentifier("args"), F.createIdentifier(arg.name), ); + case "derivedContext": { + const localName = this.getDerivedContextName(arg.path, arg.exportName); + this.ts.importUserConstruct(arg.path, arg.exportName, localName); + return F.createCallExpression( + F.createIdentifier(localName), + undefined, + arg.args.map((arg) => this.resolverParam(arg)), + ); + } + default: // @ts-expect-error throw new Error(`Unexpected resolver kind ${arg.kind}`); } } + + // Derived contexts are not anchored to anything that we know to be + // globally unique, like GraphQL type names, so must ensure this name is + // unique within our module. However, we want to avoid generating a new + // name for the same derived context more than once. + getDerivedContextName(path: string, exportName: string | null): string { + const key = `${path}:${exportName ?? ""}`; + let name = this._derivedContextNames.get(key); + if (name == null) { + name = this.ts.getUniqueName(exportName ?? "deriveContext"); + this._derivedContextNames.set(key, name); + } + return name; + } + // If a field is smantically non-null, we need to wrap the resolver in a // runtime check to ensure that the resolver does not return null. maybeApplySemanticNullRuntimeCheck( diff --git a/src/lib.ts b/src/lib.ts index cc89a4aa..9a3d4d27 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -90,7 +90,11 @@ export function extractSchemaAndDoc( const { typesWithTypename } = snapshot; const config = options.raw.grats; const checker = program.getTypeChecker(); - const ctx = TypeContext.fromSnapshot(checker, snapshot); + const ctxResult = TypeContext.fromSnapshot(checker, snapshot); + if (ctxResult.kind === "ERROR") { + return ctxResult; + } + const ctx = ctxResult.value; // Collect validation errors const validationResult = concatResults( @@ -177,6 +181,7 @@ function combineSnapshots(snapshots: ExtractionSnapshot[]): ExtractionSnapshot { const result: ExtractionSnapshot = { definitions: [], nameDefinitions: new Map(), + implicitNameDefinitions: new Map(), unresolvedNames: new Map(), typesWithTypename: new Set(), interfaceDeclarations: [], @@ -195,6 +200,10 @@ function combineSnapshots(snapshots: ExtractionSnapshot[]): ExtractionSnapshot { result.unresolvedNames.set(node, typeName); } + for (const [node, definition] of snapshot.implicitNameDefinitions) { + result.implicitNameDefinitions.set(node, definition); + } + for (const typeName of snapshot.typesWithTypename) { result.typesWithTypename.add(typeName); } diff --git a/src/metadata.ts b/src/metadata.ts index 3969a610..0faae6ef 100644 --- a/src/metadata.ts +++ b/src/metadata.ts @@ -82,11 +82,14 @@ export type StaticMethodResolver = { arguments: ResolverArgument[] | null; }; +export type ContextArgs = ContextArgument | DerivedContextArgument; + /** An argument expected by a resolver function or method */ export type ResolverArgument = | SourceArgument | ArgumentsObjectArgument | ContextArgument + | DerivedContextArgument | InformationArgument | NamedArgument; @@ -105,6 +108,14 @@ export type ContextArgument = { kind: "context"; }; +/** A context value which is expressed as a function of the global context */ +export type DerivedContextArgument = { + kind: "derivedContext"; + path: string; // Path to the module + exportName: string | null; // Export name. If omitted, the class is the default export + args: Array; +}; + /** The GraphQL info object */ export type InformationArgument = { kind: "information"; diff --git a/src/resolverSignature.ts b/src/resolverSignature.ts index 34c52a13..390e7508 100644 --- a/src/resolverSignature.ts +++ b/src/resolverSignature.ts @@ -60,6 +60,14 @@ export type ContextResolverArgument = { node: ts.Node; }; +export type DerivedContextResolverArgument = { + kind: "derivedContext"; + path: string; + exportName: string | null; + args: Array; + node: ts.Node; +}; + export type InformationResolverArgument = { kind: "information"; node: ts.Node; @@ -82,6 +90,7 @@ export type ResolverArgument = | SourceResolverArgument | ArgumentsObjectResolverArgument | ContextResolverArgument + | DerivedContextResolverArgument | InformationResolverArgument | NamedResolverArgument | UnresolvedResolverArgument; diff --git a/src/tests/fixtures/derived_context/derivedContextChain.ts b/src/tests/fixtures/derived_context/derivedContextChain.ts new file mode 100644 index 00000000..72d0b274 --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextChain.ts @@ -0,0 +1,46 @@ +/** @gqlContext */ +type RootContext = { userName: string }; + +type DerivedContextA = { greeting: string }; + +/** @gqlContext */ +export function createDerivedContextA(ctx: RootContext): DerivedContextA { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +type DerivedContextB = { greeting: string }; + +/** @gqlContext */ +export function createDerivedContextB(ctx: DerivedContextA): DerivedContextB { + return { greeting: ctx.greeting.toUpperCase() }; +} + +type EverythingContext = { greeting: string }; + +/** @gqlContext */ +export function allTheContexts( + root: RootContext, + a: DerivedContextA, + b: DerivedContextB, +): EverythingContext { + return { greeting: `${root.userName} ${a.greeting} ${b.greeting}` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: EverythingContext): string { + return ctx.greeting; +} + +/** @gqlField */ +export function consumingMultipleContexts( + _: Query, + root: RootContext, + a: DerivedContextA, + b: DerivedContextB, + everything: EverythingContext, +): string { + return `${root.userName} ${a.greeting} ${b.greeting} ${everything.greeting}`; +} diff --git a/src/tests/fixtures/derived_context/derivedContextChain.ts.expected b/src/tests/fixtures/derived_context/derivedContextChain.ts.expected new file mode 100644 index 00000000..339e9691 --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextChain.ts.expected @@ -0,0 +1,88 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { userName: string }; + +type DerivedContextA = { greeting: string }; + +/** @gqlContext */ +export function createDerivedContextA(ctx: RootContext): DerivedContextA { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +type DerivedContextB = { greeting: string }; + +/** @gqlContext */ +export function createDerivedContextB(ctx: DerivedContextA): DerivedContextB { + return { greeting: ctx.greeting.toUpperCase() }; +} + +type EverythingContext = { greeting: string }; + +/** @gqlContext */ +export function allTheContexts( + root: RootContext, + a: DerivedContextA, + b: DerivedContextB, +): EverythingContext { + return { greeting: `${root.userName} ${a.greeting} ${b.greeting}` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: EverythingContext): string { + return ctx.greeting; +} + +/** @gqlField */ +export function consumingMultipleContexts( + _: Query, + root: RootContext, + a: DerivedContextA, + b: DerivedContextB, + everything: EverythingContext, +): string { + return `${root.userName} ${a.greeting} ${b.greeting} ${everything.greeting}`; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +type Query { + consumingMultipleContexts: String + greeting: String +} +-- TypeScript -- +import { consumingMultipleContexts as queryConsumingMultipleContextsResolver, createDerivedContextA as createDerivedContextA, createDerivedContextB as createDerivedContextB, allTheContexts as allTheContexts, greeting as queryGreetingResolver } from "./derivedContextChain"; +import { GraphQLSchema, GraphQLObjectType, GraphQLString } from "graphql"; +export function getSchema(): GraphQLSchema { + const QueryType: GraphQLObjectType = new GraphQLObjectType({ + name: "Query", + fields() { + return { + consumingMultipleContexts: { + name: "consumingMultipleContexts", + type: GraphQLString, + resolve(source, _args, context) { + return queryConsumingMultipleContextsResolver(source, context, createDerivedContextA(context), createDerivedContextB(createDerivedContextA(context)), allTheContexts(context, createDerivedContextA(context), createDerivedContextB(createDerivedContextA(context)))); + } + }, + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return queryGreetingResolver(source, allTheContexts(context, createDerivedContextA(context), createDerivedContextB(createDerivedContextA(context)))); + } + } + }; + } + }); + return new GraphQLSchema({ + query: QueryType, + types: [QueryType] + }); +} diff --git a/src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts b/src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts new file mode 100644 index 00000000..4d936d67 --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts @@ -0,0 +1,26 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function greetingContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +/** @gqlField */ +export function farewell(_: Query, ctx: DerivedContext): string { + return `${ctx.greeting}... NOT!`; +} diff --git a/src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts.expected b/src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts.expected new file mode 100644 index 00000000..ce806c75 --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextUsedMultipleTimes.ts.expected @@ -0,0 +1,68 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function greetingContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +/** @gqlField */ +export function farewell(_: Query, ctx: DerivedContext): string { + return `${ctx.greeting}... NOT!`; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +type Query { + farewell: String + greeting: String +} +-- TypeScript -- +import { farewell as queryFarewellResolver, greetingContext as greetingContext, greeting as queryGreetingResolver } from "./derivedContextUsedMultipleTimes"; +import { GraphQLSchema, GraphQLObjectType, GraphQLString } from "graphql"; +export function getSchema(): GraphQLSchema { + const QueryType: GraphQLObjectType = new GraphQLObjectType({ + name: "Query", + fields() { + return { + farewell: { + name: "farewell", + type: GraphQLString, + resolve(source) { + return queryFarewellResolver(source, greetingContext(context)); + } + }, + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return queryGreetingResolver(source, greetingContext(context)); + } + } + }; + } + }); + return new GraphQLSchema({ + query: QueryType, + types: [QueryType] + }); +} diff --git a/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts b/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts new file mode 100644 index 00000000..dbadcd50 --- /dev/null +++ b/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts @@ -0,0 +1,26 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlContext */ +export function createAnotherDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Goodbye!, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected b/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected new file mode 100644 index 00000000..9af48458 --- /dev/null +++ b/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected @@ -0,0 +1,50 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlContext */ +export function createAnotherDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Goodbye!, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:6:1 - error: Duplicate derived contexts for given type + +6 type DerivedContext = { + ~~~~~~~~~~~~~~~~~~~~~~~ +7 greeting: string; + ~~~~~~~~~~~~~~~~~~~ +8 }; + ~~ + + src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:16:64 + 16 export function createAnotherDerivedContext(ctx: RootContext): DerivedContext { + ~~~~~~~~~~~~~~ + One was defined here + src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:10:5 + 10 /** @gqlContext */ + ~~~~~~~~~~~~ + Other here diff --git a/src/tests/fixtures/derived_context/simpleDerivedContext.ts b/src/tests/fixtures/derived_context/simpleDerivedContext.ts new file mode 100644 index 00000000..7312aa05 --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContext.ts @@ -0,0 +1,21 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/simpleDerivedContext.ts.expected b/src/tests/fixtures/derived_context/simpleDerivedContext.ts.expected new file mode 100644 index 00000000..a16c9f9f --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContext.ts.expected @@ -0,0 +1,55 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +type Query { + greeting: String +} +-- TypeScript -- +import { greeting as queryGreetingResolver, createDerivedContext as createDerivedContext } from "./simpleDerivedContext"; +import { GraphQLSchema, GraphQLObjectType, GraphQLString } from "graphql"; +export function getSchema(): GraphQLSchema { + const QueryType: GraphQLObjectType = new GraphQLObjectType({ + name: "Query", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return queryGreetingResolver(source, createDerivedContext(context)); + } + } + }; + } + }); + return new GraphQLSchema({ + query: QueryType, + types: [QueryType] + }); +} diff --git a/src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts b/src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts new file mode 100644 index 00000000..d0caa258 --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts @@ -0,0 +1,21 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +// type DerivedContext = { +// greeting: string; +// }; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts.expected b/src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts.expected new file mode 100644 index 00000000..62c18bbe --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts.expected @@ -0,0 +1,32 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +// type DerivedContext = { +// greeting: string; +// }; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/simpleDerivedContextUndefinedType.invalid.ts:11:57 - error: Unable to resolve type reference. In order to generate a GraphQL schema, Grats needs to determine which GraphQL type is being referenced. This requires being able to resolve type references to their `@gql` annotated declaration. However this reference could not be resolved. Is it possible that this type is not defined in this file? + +11 export function createDerivedContext(ctx: RootContext): DerivedContext { + ~~~~~~~~~~~~~~ diff --git a/src/transforms/makeResolverSignature.ts b/src/transforms/makeResolverSignature.ts index a97e7507..1770cbaa 100644 --- a/src/transforms/makeResolverSignature.ts +++ b/src/transforms/makeResolverSignature.ts @@ -4,8 +4,9 @@ import { ResolverDefinition, Metadata, FieldDefinition, + ContextArgs, } from "../metadata"; -import { nullThrows } from "../utils/helpers"; +import { invariant, nullThrows } from "../utils/helpers"; import { ResolverArgument as DirectiveResolverArgument } from "../resolverSignature"; export function makeResolverSignature(documentAst: DocumentNode): Metadata { @@ -78,23 +79,39 @@ function transformArgs( if (args == null) { return null; } - return args.map((arg): ResolverArgument => { - switch (arg.kind) { - case "argumentsObject": - return { kind: "argumentsObject" }; - case "named": - return { kind: "named", name: arg.name }; - case "source": - return { kind: "source" }; - case "information": - return { kind: "information" }; - case "context": - return { kind: "context" }; - case "unresolved": - throw new Error("Unresolved argument in resolver"); - default: - // @ts-expect-error - throw new Error(`Unknown argument kind: ${arg.kind}`); - } - }); + return args.map(transformArg); +} + +function transformArg(arg: DirectiveResolverArgument): ResolverArgument { + switch (arg.kind) { + case "argumentsObject": + return { kind: "argumentsObject" }; + case "named": + return { kind: "named", name: arg.name }; + case "source": + return { kind: "source" }; + case "information": + return { kind: "information" }; + case "context": + return { kind: "context" }; + case "derivedContext": + return { + kind: "derivedContext", + path: arg.path, + exportName: arg.exportName, + args: arg.args.map((arg): ContextArgs => { + const newArg = transformArg(arg); + invariant( + newArg.kind === "derivedContext" || newArg.kind === "context", + "Previous validation passes ensure we only have valid derived context args here", + ); + return newArg; + }), + }; + case "unresolved": + throw new Error("Unresolved argument in resolver"); + default: + // @ts-expect-error + throw new Error(`Unknown argument kind: ${arg.kind}`); + } } diff --git a/src/transforms/resolveResolverParams.ts b/src/transforms/resolveResolverParams.ts index 525522f9..f9e63337 100644 --- a/src/transforms/resolveResolverParams.ts +++ b/src/transforms/resolveResolverParams.ts @@ -1,3 +1,4 @@ +import * as ts from "typescript"; import { DefinitionNode, FieldDefinitionNode, @@ -5,7 +6,11 @@ import { Kind, visit, } from "graphql"; -import { TypeContext, UNRESOLVED_REFERENCE_NAME } from "../TypeContext"; +import { + DerivedResolverDefinition, + TypeContext, + UNRESOLVED_REFERENCE_NAME, +} from "../TypeContext"; import { err, ok } from "../utils/Result"; import { DiagnosticsResult, @@ -15,6 +20,8 @@ import { } from "../utils/DiagnosticError"; import { nullThrows } from "../utils/helpers"; import { + ContextResolverArgument, + DerivedContextResolverArgument, NamedResolverArgument, ResolverArgument, UnresolvedResolverArgument, @@ -108,6 +115,7 @@ class ResolverParamsResolver { case "argumentsObject": case "information": case "context": + case "derivedContext": case "source": return param; case "unresolved": { @@ -124,6 +132,8 @@ class ResolverParamsResolver { return param; } switch (resolved.value.kind) { + case "DERIVED_CONTEXT": + return this.resolveDerivedContext(param.node, resolved.value); case "CONTEXT": return { kind: "context", node: param.node }; case "INFO": @@ -144,6 +154,33 @@ class ResolverParamsResolver { } } } + private resolveDerivedContext( + node: ts.Node, + { path, exportName, args }: DerivedResolverDefinition, + ): ResolverArgument { + const newArgs: Array< + DerivedContextResolverArgument | ContextResolverArgument + > = []; + for (const arg of args) { + const resolvedArg = this.transformParam(arg); + switch (resolvedArg.kind) { + case "context": + case "derivedContext": + newArgs.push(resolvedArg); + break; + default: + // FIXME: Improve this error message + this.errors.push( + tsErr( + resolvedArg.node, + "Invalid argument passed to derived context function", + ), + ); + } + } + return { kind: "derivedContext", node, path, exportName, args: newArgs }; + } + resolveToPositionalArg( unresolved: UnresolvedResolverArgument, ): ResolverArgument | null { From b861b6a1892b0644545f42a9037e003780417441 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 15 Dec 2024 22:16:11 -0800 Subject: [PATCH 2/3] Document derived context --- src/Errors.ts | 16 +++ src/Extractor.ts | 5 +- src/TypeContext.ts | 13 +- .../cyclicContextDependency.invalid.ts | 24 ++++ ...yclicContextDependency.invalid.ts.expected | 40 +++++++ ...yclicContextDependencyWithChain.invalid.ts | 39 ++++++ ...extDependencyWithChain.invalid.ts.expected | 63 ++++++++++ .../derivedContextChain.ts.expected | 69 ++++++----- .../derivedContextNoReturnType.invalid.ts | 21 ++++ ...vedContextNoReturnType.invalid.ts.expected | 36 ++++++ ...erivedContextNonNamedReturnType.invalid.ts | 21 ++++ ...textNonNamedReturnType.invalid.ts.expected | 32 +++++ ...erivedContextsSameType.invalid.ts.expected | 10 +- .../simpleDerivedContextNoArgs.ts | 21 ++++ .../simpleDerivedContextNoArgs.ts.expected | 55 +++++++++ ...leDerivedContextReadsRandomType.invalid.ts | 24 ++++ ...ContextReadsRandomType.invalid.ts.expected | 35 ++++++ src/transforms/resolveResolverParams.ts | 112 +++++++++++++++--- website/docs/04-docblock-tags/04-context.mdx | 41 ++++++- 19 files changed, 609 insertions(+), 68 deletions(-) create mode 100644 src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts create mode 100644 src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts.expected create mode 100644 src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts create mode 100644 src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts.expected create mode 100644 src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts create mode 100644 src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts.expected create mode 100644 src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts create mode 100644 src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts.expected create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts.expected create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts create mode 100644 src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts.expected diff --git a/src/Errors.ts b/src/Errors.ts index e956f89e..058ac5f4 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -607,3 +607,19 @@ export function noTypesDefined() { export function tsConfigNotFound(cwd: string) { return `Grats: Could not find \`tsconfig.json\` searching in ${cwd}.\n\nSee https://www.typescriptlang.org/download/ for instructors on how to add TypeScript to your project. Then run \`npx tsc --init\` to create a \`tsconfig.json\` file.`; } + +export function cyclicDerivedContext() { + return `Cyclic dependency detected in derived context. This derived context value depends upon itself.`; +} + +export function invalidDerivedContextArgType() { + return "Invalid type for derived context function argument. Derived context functions may only accept other `@gqlContext` types as arguments."; +} + +export function missingReturnTypeForDerivedResolver() { + return 'Expected derived resolver to have an explicit return type. This is needed to allow Grats to "see" which type to treat as a derived context type.'; +} + +export function derivedResolverInvalidReturnType() { + return "Expected derived resolver function's return type to be a type reference. Grats uses this type reference to determine which type to treat as a derived context type."; +} diff --git a/src/Extractor.ts b/src/Extractor.ts index 27ec539a..08ec52a7 100644 --- a/src/Extractor.ts +++ b/src/Extractor.ts @@ -139,7 +139,6 @@ class Extractor { name: NameNode, kind: NameDefinition["kind"], ): void { - // @ts-ignore FIXME this.nameDefinitions.set(node, { name, kind }); } @@ -341,10 +340,10 @@ class Extractor { recordDerivedContext(node: ts.FunctionDeclaration, tag: ts.JSDocTag) { const returnType = node.type; if (returnType == null) { - throw new Error("Function declaration must have a return type"); + return this.report(node, E.missingReturnTypeForDerivedResolver()); } if (!ts.isTypeReferenceNode(returnType)) { - throw new Error("Function declaration must return an explicit type"); + return this.report(returnType, E.missingReturnTypeForDerivedResolver()); } const funcName = this.namedFunctionExportName(node); diff --git a/src/TypeContext.ts b/src/TypeContext.ts index 9647f56d..8b3dee1c 100644 --- a/src/TypeContext.ts +++ b/src/TypeContext.ts @@ -87,11 +87,14 @@ export class TypeContext { const existing = self._declarationToName.get(declaration); if (existing != null) { errors.push( - // TODO: Better error messages here - tsErr(declaration, "Duplicate derived contexts for given type", [ - tsRelated(reference, "One was defined here"), - gqlRelated(existing.name, "Other here"), - ]), + tsErr( + declaration, + "Multiple derived contexts defined for given type", + [ + gqlRelated(definition.name, "One was defined here"), + gqlRelated(existing.name, "Another here"), + ], + ), ); continue; } diff --git a/src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts b/src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts new file mode 100644 index 00000000..046d4cb3 --- /dev/null +++ b/src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts @@ -0,0 +1,24 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext( + ctx: RootContext, + oops: DerivedContext, +): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts.expected b/src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts.expected new file mode 100644 index 00000000..bba94fbe --- /dev/null +++ b/src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts.expected @@ -0,0 +1,40 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext( + ctx: RootContext, + oops: DerivedContext, +): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts:10:5 - error: Cyclic dependency detected in derived context. This derived context value depends upon itself. + +10 /** @gqlContext */ + ~~~~~~~~~~~~ + + src/tests/fixtures/derived_context/cyclicContextDependency.invalid.ts:13:3 + 13 oops: DerivedContext, + ~~~~~~~~~~~~~~~~~~~~ + This derived context depends on itself diff --git a/src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts b/src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts new file mode 100644 index 00000000..b9c63388 --- /dev/null +++ b/src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts @@ -0,0 +1,39 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type A = { + greeting: string; +}; + +/** @gqlContext */ +export function a(ctx: RootContext, b: B): A { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +type B = { + greeting: string; +}; + +/** @gqlContext */ +export function b(ctx: RootContext, c: C): B { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +type C = { + greeting: string; +}; + +/** @gqlContext */ +export function c(ctx: RootContext, a: A): C { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: A): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts.expected b/src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts.expected new file mode 100644 index 00000000..18e441f9 --- /dev/null +++ b/src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts.expected @@ -0,0 +1,63 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type A = { + greeting: string; +}; + +/** @gqlContext */ +export function a(ctx: RootContext, b: B): A { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +type B = { + greeting: string; +}; + +/** @gqlContext */ +export function b(ctx: RootContext, c: C): B { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +type C = { + greeting: string; +}; + +/** @gqlContext */ +export function c(ctx: RootContext, a: A): C { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: A): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts:10:5 - error: Cyclic dependency detected in derived context. This derived context value depends upon itself. + +10 /** @gqlContext */ + ~~~~~~~~~~~~ + + src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts:11:37 + 11 export function a(ctx: RootContext, b: B): A { + ~~~~ + This derived context depends on + src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts:20:37 + 20 export function b(ctx: RootContext, c: C): B { + ~~~~ + Which in turn depends on + src/tests/fixtures/derived_context/cyclicContextDependencyWithChain.invalid.ts:29:37 + 29 export function c(ctx: RootContext, a: A): C { + ~~~~ + Which ultimately creates a cycle back to the initial derived context diff --git a/src/tests/fixtures/derived_context/derivedContextChain.ts.expected b/src/tests/fixtures/derived_context/derivedContextChain.ts.expected index 339e9691..8d2e985d 100644 --- a/src/tests/fixtures/derived_context/derivedContextChain.ts.expected +++ b/src/tests/fixtures/derived_context/derivedContextChain.ts.expected @@ -51,38 +51,37 @@ export function consumingMultipleContexts( ----------------- OUTPUT ----------------- --- SDL -- -type Query { - consumingMultipleContexts: String - greeting: String -} --- TypeScript -- -import { consumingMultipleContexts as queryConsumingMultipleContextsResolver, createDerivedContextA as createDerivedContextA, createDerivedContextB as createDerivedContextB, allTheContexts as allTheContexts, greeting as queryGreetingResolver } from "./derivedContextChain"; -import { GraphQLSchema, GraphQLObjectType, GraphQLString } from "graphql"; -export function getSchema(): GraphQLSchema { - const QueryType: GraphQLObjectType = new GraphQLObjectType({ - name: "Query", - fields() { - return { - consumingMultipleContexts: { - name: "consumingMultipleContexts", - type: GraphQLString, - resolve(source, _args, context) { - return queryConsumingMultipleContextsResolver(source, context, createDerivedContextA(context), createDerivedContextB(createDerivedContextA(context)), allTheContexts(context, createDerivedContextA(context), createDerivedContextB(createDerivedContextA(context)))); - } - }, - greeting: { - name: "greeting", - type: GraphQLString, - resolve(source) { - return queryGreetingResolver(source, allTheContexts(context, createDerivedContextA(context), createDerivedContextB(createDerivedContextA(context)))); - } - } - }; - } - }); - return new GraphQLSchema({ - query: QueryType, - types: [QueryType] - }); -} +src/tests/fixtures/derived_context/derivedContextChain.ts:6:5 - error: Cyclic dependency detected in derived context. This derived context value depends upon itself. + +6 /** @gqlContext */ + ~~~~~~~~~~~~ + + src/tests/fixtures/derived_context/derivedContextChain.ts:23:3 + 23 a: DerivedContextA, + ~~~~~~~~~~~~~~~~~~ + This derived context depends on + src/tests/fixtures/derived_context/derivedContextChain.ts:24:3 + 24 b: DerivedContextB, + ~~~~~~~~~~~~~~~~~~ + Which in turn depends on + src/tests/fixtures/derived_context/derivedContextChain.ts:14:39 + 14 export function createDerivedContextB(ctx: DerivedContextA): DerivedContextB { + ~~~~~~~~~~~~~~~~~~~~ + Which ultimately creates a cycle back to the initial derived context +src/tests/fixtures/derived_context/derivedContextChain.ts:6:5 - error: Cyclic dependency detected in derived context. This derived context value depends upon itself. + +6 /** @gqlContext */ + ~~~~~~~~~~~~ + + src/tests/fixtures/derived_context/derivedContextChain.ts:23:3 + 23 a: DerivedContextA, + ~~~~~~~~~~~~~~~~~~ + This derived context depends on + src/tests/fixtures/derived_context/derivedContextChain.ts:24:3 + 24 b: DerivedContextB, + ~~~~~~~~~~~~~~~~~~ + Which in turn depends on + src/tests/fixtures/derived_context/derivedContextChain.ts:14:39 + 14 export function createDerivedContextB(ctx: DerivedContextA): DerivedContextB { + ~~~~~~~~~~~~~~~~~~~~ + Which ultimately creates a cycle back to the initial derived context diff --git a/src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts b/src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts new file mode 100644 index 00000000..361fc220 --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts @@ -0,0 +1,21 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext) { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts.expected b/src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts.expected new file mode 100644 index 00000000..2de2078e --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts.expected @@ -0,0 +1,36 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext) { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/derivedContextNoReturnType.invalid.ts:11:1 - error: Expected derived resolver to have an explicit return type. This is needed to allow Grats to "see" which type to treat as a derived context type. + +11 export function createDerivedContext(ctx: RootContext) { + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +12 return { greeting: `Hello, ${ctx.userName}!` }; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +13 } + ~ diff --git a/src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts b/src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts new file mode 100644 index 00000000..01419dae --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts @@ -0,0 +1,21 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): { greeting: string } { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts.expected b/src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts.expected new file mode 100644 index 00000000..1a068831 --- /dev/null +++ b/src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts.expected @@ -0,0 +1,32 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(ctx: RootContext): { greeting: string } { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/derivedContextNonNamedReturnType.invalid.ts:11:57 - error: Expected derived resolver to have an explicit return type. This is needed to allow Grats to "see" which type to treat as a derived context type. + +11 export function createDerivedContext(ctx: RootContext): { greeting: string } { + ~~~~~~~~~~~~~~~~~~~~ diff --git a/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected b/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected index 9af48458..c1295530 100644 --- a/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected +++ b/src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts.expected @@ -31,7 +31,7 @@ export function greeting(_: Query, ctx: DerivedContext): string { ----------------- OUTPUT ----------------- -src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:6:1 - error: Duplicate derived contexts for given type +src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:6:1 - error: Multiple derived contexts defined for given type 6 type DerivedContext = { ~~~~~~~~~~~~~~~~~~~~~~~ @@ -40,11 +40,11 @@ src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:6: 8 }; ~~ - src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:16:64 - 16 export function createAnotherDerivedContext(ctx: RootContext): DerivedContext { - ~~~~~~~~~~~~~~ + src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:15:5 + 15 /** @gqlContext */ + ~~~~~~~~~~~~ One was defined here src/tests/fixtures/derived_context/multipleDerivedContextsSameType.invalid.ts:10:5 10 /** @gqlContext */ ~~~~~~~~~~~~ - Other here + Another here diff --git a/src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts b/src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts new file mode 100644 index 00000000..f8cadd21 --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts @@ -0,0 +1,21 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(): DerivedContext { + return { greeting: `Hello!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts.expected b/src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts.expected new file mode 100644 index 00000000..a98d979d --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContextNoArgs.ts.expected @@ -0,0 +1,55 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext(): DerivedContext { + return { greeting: `Hello!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +type Query { + greeting: String +} +-- TypeScript -- +import { greeting as queryGreetingResolver, createDerivedContext as createDerivedContext } from "./simpleDerivedContextNoArgs"; +import { GraphQLSchema, GraphQLObjectType, GraphQLString } from "graphql"; +export function getSchema(): GraphQLSchema { + const QueryType: GraphQLObjectType = new GraphQLObjectType({ + name: "Query", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return queryGreetingResolver(source, createDerivedContext()); + } + } + }; + } + }); + return new GraphQLSchema({ + query: QueryType, + types: [QueryType] + }); +} diff --git a/src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts b/src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts new file mode 100644 index 00000000..b7a73c06 --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts @@ -0,0 +1,24 @@ +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext( + ctx: RootContext, + oops: string, +): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} diff --git a/src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts.expected b/src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts.expected new file mode 100644 index 00000000..c571a491 --- /dev/null +++ b/src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts.expected @@ -0,0 +1,35 @@ +----------------- +INPUT +----------------- +/** @gqlContext */ +type RootContext = { + userName: string; +}; + +type DerivedContext = { + greeting: string; +}; + +/** @gqlContext */ +export function createDerivedContext( + ctx: RootContext, + oops: string, +): DerivedContext { + return { greeting: `Hello, ${ctx.userName}!` }; +} + +/** @gqlType */ +type Query = unknown; + +/** @gqlField */ +export function greeting(_: Query, ctx: DerivedContext): string { + return ctx.greeting; +} + +----------------- +OUTPUT +----------------- +src/tests/fixtures/derived_context/simpleDerivedContextReadsRandomType.invalid.ts:13:3 - error: Invalid type for derived context function argument. Derived context functions may only accept other `@gqlContext` types as arguments. + +13 oops: string, + ~~~~~~~~~~~~ diff --git a/src/transforms/resolveResolverParams.ts b/src/transforms/resolveResolverParams.ts index f9e63337..5258e117 100644 --- a/src/transforms/resolveResolverParams.ts +++ b/src/transforms/resolveResolverParams.ts @@ -15,10 +15,11 @@ import { err, ok } from "../utils/Result"; import { DiagnosticsResult, FixableDiagnosticWithLocation, + gqlErr, tsErr, tsRelated, } from "../utils/DiagnosticError"; -import { nullThrows } from "../utils/helpers"; +import { invariant, nullThrows } from "../utils/helpers"; import { ContextResolverArgument, DerivedContextResolverArgument, @@ -69,9 +70,12 @@ class ResolverParamsResolver { } // Resolve all the params individually - const resolverParams: ResolverArgument[] = resolver.arguments.map((param) => - this.transformParam(param), - ); + const resolverParams: ResolverArgument[] = []; + for (const param of resolver.arguments) { + const transformed = this.transformParam(param); + if (transformed == null) return field; + resolverParams.push(transformed); + } // Now we check to see if the params are a valid combination... const args = resolverParams.find( @@ -109,7 +113,10 @@ class ResolverParamsResolver { return { ...field, arguments: fieldArgs, resolver: newResolver }; } - transformParam(param: ResolverArgument): ResolverArgument { + transformParam( + param: ResolverArgument, + seenDerivedContextValues?: Map, + ): ResolverArgument | null { switch (param.kind) { case "named": case "argumentsObject": @@ -129,11 +136,18 @@ class ResolverParamsResolver { ); if (resolved.kind === "ERROR") { this.errors.push(resolved.err); - return param; + return null; } switch (resolved.value.kind) { - case "DERIVED_CONTEXT": - return this.resolveDerivedContext(param.node, resolved.value); + case "DERIVED_CONTEXT": { + const derivedContextArg = this.resolveDerivedContext( + param.node, + resolved.value, + seenDerivedContextValues, + ); + if (derivedContextArg === null) return null; + return derivedContextArg; + } case "CONTEXT": return { kind: "context", node: param.node }; case "INFO": @@ -154,27 +168,48 @@ class ResolverParamsResolver { } } } + private resolveDerivedContext( - node: ts.Node, - { path, exportName, args }: DerivedResolverDefinition, - ): ResolverArgument { + node: ts.Node, // Argument + definition: DerivedResolverDefinition, + seenDerivedContextValues?: Map, + ): ResolverArgument | null { + const { path, exportName, args } = definition; + const key = `${path}:${exportName}`; + if (seenDerivedContextValues == null) { + // We're resolving the arg of a resolver. Initiate the map. + seenDerivedContextValues = new Map(); + } else { + if (seenDerivedContextValues.has(key)) { + this.errors.push( + this.cycleError(node, definition, seenDerivedContextValues), + ); + return null; + } + } + seenDerivedContextValues.set(key, node); + const newArgs: Array< DerivedContextResolverArgument | ContextResolverArgument > = []; for (const arg of args) { - const resolvedArg = this.transformParam(arg); + const resolvedArg = this.transformParam(arg, seenDerivedContextValues); + if (resolvedArg === null) { + continue; + } switch (resolvedArg.kind) { case "context": + newArgs.push(resolvedArg); + break; case "derivedContext": + // Here we know that the argument `node` maps to a derived context + // `definition` which itself depends another derived resolver `resolvedArg`. + // `definition`. newArgs.push(resolvedArg); break; default: - // FIXME: Improve this error message this.errors.push( - tsErr( - resolvedArg.node, - "Invalid argument passed to derived context function", - ), + tsErr(resolvedArg.node, E.invalidDerivedContextArgType()), ); } } @@ -198,4 +233,47 @@ class ResolverParamsResolver { node: unresolved.node, }; } + + /** + * Some slightly complicated logic to construct nice errors in the case of + * cycles where derived resolvers ultimately depend upon themselves. + * + * The `@gqlContext` tag is the main location. If it's a direct cycle, we + * report one related location, of the argument which points back to itself. + * + * If there are multiple nodes in the cycle, we report a related location for + * each node in the cycle, with a message that depends on the position of the + * node in the cycle. + */ + cycleError( + node: ts.Node, + definition: DerivedResolverDefinition, + seenDerivedContextValues: Map, + ): ts.DiagnosticWithLocation { + // We trim off the first node because that points to a resolver argument. + const nodes = Array.from(seenDerivedContextValues.values()).slice(1); + // The cycle completes with this node, so we include it in the list. + nodes.push(node); + const related = nodes.map((def, i) => { + if (nodes.length === 1) { + return tsRelated(def, "This derived context depends on itself"); + } + + const isFirst = i === 0; + const isLast = i === nodes.length - 1; + + invariant(!(isFirst && isLast), "Should not be both first and last"); + + if (isFirst) { + return tsRelated(def, "This derived context depends on"); + } else if (!isLast) { + return tsRelated(def, "Which in turn depends on"); + } + return tsRelated( + def, + "Which ultimately creates a cycle back to the initial derived context", + ); + }); + return gqlErr(definition.name, E.cyclicDerivedContext(), related); + } } diff --git a/website/docs/04-docblock-tags/04-context.mdx b/website/docs/04-docblock-tags/04-context.mdx index 3bbd04c3..27987a16 100644 --- a/website/docs/04-docblock-tags/04-context.mdx +++ b/website/docs/04-docblock-tags/04-context.mdx @@ -13,6 +13,8 @@ type GQLCtx = { }; ``` +## Consuming context values in resolvers + Grats will detect any resolver parameter that is typed using the `@gqlContext` type and ensure that the context object is passed to the resolver in that position: ```ts @@ -37,10 +39,43 @@ export function me(ctx: GQLCtx): User { Unlike `graphql-js`, Grats does not require that the context object be passed as a specific positional argument to the resolver. You can place the context object anywhere in the argument list, as long as the type is annotated with the `@gqlContext` type. ::: -## Constructing your context object +## Derived context values + +In some cases you may have a context value that is expensive to create, or is only used in some resolvers. In these cases you can define a _derived resolver_. A derived resolver is a function which produces a context value. It may optionally read, as arguments, other context values, including other derived context values. + +Once a derived resolver is defined, the type it returns becomes a new context type. **Any resolver argument with this type will receive the value produced by the derived resolver returning that type.** + +:::tip +Derived context functions will be called individually by each resolve that wants to access the derived context value. If you want the result to be reused across multiple resolvers, you should memoize the result. +::: + +```ts +/** + * A resolver which reads both the global context and a derived context value. + * + * @gqlQueryContext */ +export function me(ctx: GQLCtx, db: Database): User { + return db.users.getById(ctx.userID); +} + +// A derived context resolver cached using a WeakMap to ensure the value is +// computed at most once per request. + +const DB_CACHE = new WeakMap(); + +/** @gqlContext */ +export function createDb(ctx: GQLCtx): Database { + if (!DB_CACHE.has(ctx)) { + DB_CACHE.set(ctx, new Database()); + } + return DB_CACHE.get(ctx); +} +``` + +## Constructing your root context object -The mechanism by which you construct your context object will vary depending upon the GraphQL server library you are using. See your GraphQL server library's documentation for more information. +How you construct your context object will vary based on your GraphQL server library. See your GraphQL server library's documentation for more information. :::caution -Grats can ensure that every resolver is expecting the annotated context type, but it cannot ensure that the context value you construct and pass in matches that type. **It is up to you to ensure that your context value is constructed correctly and passed to the GraphQL execution engine.** +Grats can ensure that every resolver is expecting the same annotated root context type, but it cannot ensure that the root context value you construct and pass in matches that type. **It is up to you to ensure that your root context value is constructed correctly and passed to the GraphQL execution engine.** ::: From 331838367108e6bf008464f6b4308acf5eaf93cf Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Tue, 14 Jan 2025 17:01:00 -0800 Subject: [PATCH 3/3] Cleanup --- src/Extractor.ts | 13 +++- src/TypeContext.ts | 64 ++++++++++--------- .../validateDuplicateContextOrInfo.ts | 2 +- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/Extractor.ts b/src/Extractor.ts index 08ec52a7..6380a1ba 100644 --- a/src/Extractor.ts +++ b/src/Extractor.ts @@ -27,7 +27,11 @@ import { } from "./utils/DiagnosticError"; import { err, ok } from "./utils/Result"; import * as ts from "typescript"; -import { NameDefinition, UNRESOLVED_REFERENCE_NAME } from "./TypeContext"; +import { + DeclarationDefinition, + NameDefinition, + UNRESOLVED_REFERENCE_NAME, +} from "./TypeContext"; import * as E from "./Errors"; import { traverseJSDocTags } from "./utils/JSDoc"; import { GraphQLConstructor } from "./GraphQLConstructor"; @@ -86,7 +90,10 @@ export type ExtractionSnapshot = { readonly definitions: DefinitionNode[]; readonly unresolvedNames: Map; readonly nameDefinitions: Map; - readonly implicitNameDefinitions: Map; + readonly implicitNameDefinitions: Map< + DeclarationDefinition, + ts.TypeReferenceNode + >; readonly typesWithTypename: Set; readonly interfaceDeclarations: Array; }; @@ -118,7 +125,7 @@ class Extractor { // Snapshot data unresolvedNames: Map = new Map(); nameDefinitions: Map = new Map(); - implicitNameDefinitions: Map = + implicitNameDefinitions: Map = new Map(); typesWithTypename: Set = new Set(); interfaceDeclarations: Array = []; diff --git a/src/TypeContext.ts b/src/TypeContext.ts index 8b3dee1c..e0c7e4eb 100644 --- a/src/TypeContext.ts +++ b/src/TypeContext.ts @@ -13,7 +13,6 @@ import { gqlRelated, DiagnosticsResult, FixableDiagnosticWithLocation, - tsRelated, } from "./utils/DiagnosticError"; import { err, ok } from "./utils/Result"; import * as E from "./Errors"; @@ -30,20 +29,20 @@ export type DerivedResolverDefinition = { kind: "DERIVED_CONTEXT"; }; -export type NameDefinition = - | { - name: NameNode; - kind: - | "TYPE" - | "INTERFACE" - | "UNION" - | "SCALAR" - | "INPUT_OBJECT" - | "ENUM" - | "CONTEXT" - | "INFO"; - } - | DerivedResolverDefinition; +export type NameDefinition = { + name: NameNode; + kind: + | "TYPE" + | "INTERFACE" + | "UNION" + | "SCALAR" + | "INPUT_OBJECT" + | "ENUM" + | "CONTEXT" + | "INFO"; +}; + +export type DeclarationDefinition = NameDefinition | DerivedResolverDefinition; type TsIdentifier = number; @@ -62,7 +61,8 @@ type TsIdentifier = number; export class TypeContext { checker: ts.TypeChecker; - _declarationToName: Map = new Map(); + _declarationToDefinition: Map = + new Map(); _unresolvedNodes: Map = new Map(); _idToDeclaration: Map = new Map(); @@ -76,7 +76,7 @@ export class TypeContext { self._markUnresolvedType(node, typeName); } for (const [node, definition] of snapshot.nameDefinitions) { - self._recordTypeName(node, definition); + self._recordDeclaration(node, definition); } for (const [definition, reference] of snapshot.implicitNameDefinitions) { const declaration = self.maybeTsDeclarationForTsName(reference.typeName); @@ -84,7 +84,7 @@ export class TypeContext { errors.push(tsErr(reference.typeName, E.unresolvedTypeReference())); continue; } - const existing = self._declarationToName.get(declaration); + const existing = self._declarationToDefinition.get(declaration); if (existing != null) { errors.push( tsErr( @@ -98,8 +98,7 @@ export class TypeContext { ); continue; } - - self._recordTypeName(declaration, definition); + self._recordDeclaration(declaration, definition); } if (errors.length > 0) { @@ -114,9 +113,12 @@ export class TypeContext { // Record that a GraphQL construct of type `kind` with the name `name` is // declared at `node`. - private _recordTypeName(node: ts.Declaration, definition: NameDefinition) { + private _recordDeclaration( + node: ts.Declaration, + definition: DeclarationDefinition, + ) { this._idToDeclaration.set(definition.name.tsIdentifier, node); - this._declarationToName.set(node, definition); + this._declarationToDefinition.set(node, definition); } // Record that a type references `node` @@ -124,8 +126,8 @@ export class TypeContext { this._unresolvedNodes.set(name.tsIdentifier, node); } - allNameDefinitions(): Iterable { - return this._declarationToName.values(); + allDefinitions(): Iterable { + return this._declarationToDefinition.values(); } findSymbolDeclaration(startSymbol: ts.Symbol): ts.Declaration | null { @@ -173,7 +175,9 @@ export class TypeContext { ); } - const nameDefinition = this._declarationToName.get(declarationResult.value); + const nameDefinition = this._declarationToDefinition.get( + declarationResult.value, + ); if (nameDefinition == null) { return err(gqlErr(unresolved, E.unresolvedTypeReference())); } @@ -194,12 +198,12 @@ export class TypeContext { if (referenceNode == null) return false; const declaration = this.maybeTsDeclarationForTsName(referenceNode); if (declaration == null) return false; - return this._declarationToName.has(declaration); + return this._declarationToDefinition.has(declaration); } gqlNameDefinitionForGqlName( nameNode: NameNode, - ): DiagnosticResult { + ): DiagnosticResult { const referenceNode = this.getEntityName(nameNode); if (referenceNode == null) { throw new Error("Expected to find reference node for name node."); @@ -209,7 +213,7 @@ export class TypeContext { if (declaration == null) { return err(gqlErr(nameNode, E.unresolvedTypeReference())); } - const definition = this._declarationToName.get(declaration); + const definition = this._declarationToDefinition.get(declaration); if (definition == null) { return err(gqlErr(nameNode, E.unresolvedTypeReference())); } @@ -230,7 +234,9 @@ export class TypeContext { ); } - const nameDefinition = this._declarationToName.get(declarationResult.value); + const nameDefinition = this._declarationToDefinition.get( + declarationResult.value, + ); if (nameDefinition == null) { return err(tsErr(node, E.unresolvedTypeReference())); } diff --git a/src/validations/validateDuplicateContextOrInfo.ts b/src/validations/validateDuplicateContextOrInfo.ts index 0347a9ef..578ee784 100644 --- a/src/validations/validateDuplicateContextOrInfo.ts +++ b/src/validations/validateDuplicateContextOrInfo.ts @@ -14,7 +14,7 @@ export function validateDuplicateContextOrInfo( const errors: FixableDiagnosticWithLocation[] = []; let infoDefinition: null | NameDefinition = null; let ctxDefinition: null | NameDefinition = null; - for (const namedDefinition of ctx.allNameDefinitions()) { + for (const namedDefinition of ctx.allDefinitions()) { switch (namedDefinition.kind) { case "CONTEXT": if (ctxDefinition != null) {