Skip to content

Commit

Permalink
feat: Don't force async if connection resolvers are sync (#707)
Browse files Browse the repository at this point in the history
  • Loading branch information
tgriesser authored Dec 7, 2020
1 parent 9d0f4f4 commit 0cff220
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 48 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"jest": "^26.6.3",
"jest-watch-typeahead": "^0.6.1",
"lint-staged": "^7.3.0",
"prettier": "^2.0.5",
"prettier": "^2.2.1",
"prettier-plugin-jsdoc": "^0.2.5",
"sort-package-json": "^1.22.1",
"ts-jest": "^26.4.4",
Expand Down
29 changes: 23 additions & 6 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { NexusObjectTypeConfig, ObjectDefinitionBlock } from './definitions/obje
import { NexusSchemaExtension } from './extensions'
import { isPromiseLike, PrintedGenTyping, PrintedGenTypingImport, venn } from './utils'
import { NexusFinalArgConfig } from './definitions/args'
import { UnwrapPromise } from './typeHelpersInternal'

export { PluginBuilderLens }

Expand Down Expand Up @@ -118,22 +119,38 @@ export interface PluginConfig {
// onPrint?: (visitor: Visitor<ASTKindToNode>) => void;
}

export function completeValue<T, R>(
valOrPromise: PromiseLike<T> | T,
onSuccess: (completedVal: T) => R
): R | UnwrapPromise<R> | PromiseLike<UnwrapPromise<R>> | PromiseLike<UnwrapPromise<R>>

export function completeValue<T, R, E>(
valOrPromise: PromiseLike<T> | T,
onSuccess: (completedVal: T) => R,
onError: (err: any) => R
): R | UnwrapPromise<R> | PromiseLike<UnwrapPromise<R>> | PromiseLike<UnwrapPromise<R>>

/**
* Helper for allowing plugins to fulfill the return of the `next` resolver, without paying the cost of the
* Promise if not required.
*/
export function completeValue<T, R = T>(
export function completeValue<T, R>(
valOrPromise: PromiseLike<T> | T,
onSuccess: (completedVal: T) => R,
onError?: (errVal: any) => T
onError?: (errVal: any) => R
) {
if (isPromiseLike(valOrPromise)) {
return valOrPromise.then((completedValue) => {
return onSuccess(completedValue)
}, onError)
return valOrPromise.then(onSuccess, onError)
}
// No need to handle onError, this should just be a try/catch inside the `onSuccess` block
return onSuccess(valOrPromise)
const result = onSuccess(valOrPromise)

// If the result of the synchronous call is a promise, we want to unwrap it, for
// the return value types consistency
if (isPromiseLike(result)) {
return result.then((o) => o)
}
return result
}

export type MiddlewareFn = (
Expand Down
63 changes: 34 additions & 29 deletions src/plugins/connectionPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from '../typegenTypeHelpers'
import { eachObj, getOwnPackage, isPromiseLike, mapObj, pathToArray, printedGenTypingImport } from '../utils'
import { nullable, NexusNullDef } from '../definitions/nullable'
import { MaybePromiseLike } from '../typeHelpersInternal'

export interface ConnectionPluginConfig {
/**
Expand Down Expand Up @@ -255,8 +256,8 @@ export type ConnectionFieldConfig<TypeName extends string = any, FieldName exten
}
| {
/**
* Implement the full resolve, including `edges` and `pageInfo`. Useful for more complex pagination
* cases, where you may want to use utilities from other libraries like GraphQL Relay JS, and only use
* Implement the full resolve, including `edges` and `pageInfo`. Useful in more complex pagination
* cases, or if you want to use utilities from other libraries like GraphQL Relay JS, and only use
* Nexus for the construction and type-safety:
*
* Https://github.com/graphql/graphql-relay-js
Expand Down Expand Up @@ -670,27 +671,32 @@ export function makeResolveFn(

// Local variable to cache the execution of fetching the nodes,
// which is needed for all fields.
let cachedNodes: Promise<Array<any>>
let cachedEdges: Promise<{
edges: Array<EdgeLike | null>
let cachedNodes: MaybePromiseLike<Array<any>>
let cachedEdges: MaybePromiseLike<{
edges: EdgeLike[]
nodes: any[]
}>
let hasPromise = false

// Get all the nodes, before any pagination slicing
const resolveAllNodes = () => {
if (cachedNodes !== undefined) {
return cachedNodes
}
cachedNodes = Promise.resolve(nodesResolve(root, formattedArgs, ctx, info) || null)
return cachedNodes.then((allNodes) => (allNodes ? Array.from(allNodes) : allNodes))

cachedNodes = completeValue(nodesResolve(root, formattedArgs, ctx, info) ?? null, (allNodes) => {
return allNodes ? Array.from(allNodes) : allNodes
})

return cachedNodes
}

const resolveEdgesAndNodes = () => {
if (cachedEdges !== undefined) {
return cachedEdges
}

cachedEdges = resolveAllNodes().then((allNodes) => {
cachedEdges = completeValue(resolveAllNodes(), (allNodes) => {
if (!allNodes) {
const arrPath = JSON.stringify(pathToArray(info.path))
console.warn(
Expand All @@ -701,7 +707,6 @@ export function makeResolveFn(

const resolvedEdgeList: MaybePromise<EdgeLike>[] = []
const resolvedNodeList: any[] = []
let hasPromise = false

iterateNodes(allNodes, args, (maybeNode, i) => {
if (isPromiseLike(maybeNode)) {
Expand Down Expand Up @@ -755,32 +760,32 @@ export function makeResolveFn(
return cachedEdges
}

const resolvePageInfo = async () => {
const [allNodes, { edges }] = await Promise.all([resolveAllNodes(), resolveEdgesAndNodes()])
let basePageInfo = allNodes
? pageInfoFromNodes(allNodes, args, ctx, info)
: {
hasNextPage: false,
hasPreviousPage: false,
}

if (isPromiseLike(basePageInfo)) {
basePageInfo = await basePageInfo
}

return {
...basePageInfo,
startCursor: edges?.[0]?.cursor ? edges[0].cursor : null,
endCursor: edges?.[edges.length - 1]?.cursor ?? null,
}
const resolvePageInfo = () => {
return completeValue(resolveAllNodes(), (allNodes) =>
completeValue(resolveEdgesAndNodes(), ({ edges }) =>
completeValue(
allNodes
? pageInfoFromNodes(allNodes, args, ctx, info)
: {
hasNextPage: false,
hasPreviousPage: false,
},
(basePageInfo) => ({
...basePageInfo,
startCursor: edges?.[0]?.cursor ? edges[0].cursor : null,
endCursor: edges?.[edges.length - 1]?.cursor ?? null,
})
)
)
)
}

return {
get nodes() {
return resolveEdgesAndNodes().then((o) => o.nodes)
return completeValue(resolveEdgesAndNodes(), (o) => o.nodes)
},
get edges() {
return resolveEdgesAndNodes().then((o) => o.edges)
return completeValue(resolveEdgesAndNodes(), (o) => o.edges)
},
get pageInfo() {
return resolvePageInfo()
Expand Down
22 changes: 19 additions & 3 deletions src/typeHelpersInternal.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
/** Borrowed from `type-fest` Extract the keys from a type where the value type of the key extends the given `Condition`. */
/**
* Borrowed from `type-fest`
*
* Extract the keys from a type where the value type of the key extends the given `Condition`.
*/
export type ConditionalKeys<Base, Condition> = NonNullable<
// Wrap in `NonNullable` to strip away the `undefined` type from the produced union.
{
Expand All @@ -11,10 +15,18 @@ export type ConditionalKeys<Base, Condition> = NonNullable<
}[keyof Base]
>

/** Taken from `type-fest` Pick keys from the shape that matches the given `Condition`. */
/**
* Taken from `type-fest`
*
* Pick keys from the shape that matches the given `Condition`.
*/
export type ConditionalPick<Base, Condition> = Pick<Base, ConditionalKeys<Base, Condition>>

/** Taken from `type-fest` Get the values of a mapped types */
/**
* Taken from `type-fest`
*
* Get the values of a mapped types
*/
export type ValueOf<ObjectType, ValueType extends keyof ObjectType = keyof ObjectType> = ObjectType[ValueType]

/** Is the given type equal to the other given type? */
Expand All @@ -40,3 +52,7 @@ type DoRequireDeeply<T> = {
? DoRequireDeeply<Exclude<T[K], undefined>>
: Exclude<T[K], undefined>
}

export type MaybePromiseLike<T> = T | PromiseLike<T>

export type UnwrapPromise<R> = R extends PromiseLike<infer U> ? U : R
8 changes: 3 additions & 5 deletions src/typegenAbstractTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,9 @@ export type AbstractTypeNames<TypeName extends string> = ConditionalKeys<
>

/** Returns whether all the abstract type names where TypeName is used have implemented `resolveType` */
export type IsStrategyResolveTypeImplementedInAllAbstractTypes<TypeName extends string> = AbstractTypeNames<
TypeName
> extends GetGen<'abstractsUsingStrategyResolveType'>
? true
: false
export type IsStrategyResolveTypeImplementedInAllAbstractTypes<
TypeName extends string
> = AbstractTypeNames<TypeName> extends GetGen<'abstractsUsingStrategyResolveType'> ? true : false

/** Returns whether all the members of an abstract type have implemented `isTypeOf` */
export type IsStrategyIsTypeOfImplementedInAllMembers<AbstractTypeName extends string> = GetGen2<
Expand Down
24 changes: 24 additions & 0 deletions tests/v15/__snapshots__/connectionPlugin-v15.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`field level configuration does not pay the cost of async unless there is an async call 1`] = `
Object {
"data": Object {
"users": Object {
"edges": Array [
Object {
"cursor": "Y3Vyc29yOjA=",
"node": Object {
"id": "User:1",
},
},
],
"pageInfo": Object {
"endCursor": "Y3Vyc29yOjA=",
"hasNextPage": true,
"hasPreviousPage": false,
"startCursor": "Y3Vyc29yOjA=",
},
},
},
}
`;
83 changes: 83 additions & 0 deletions tests/v15/connectionPlugin-v15.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { executeSync, parse } from 'graphql'
import { connectionPlugin, makeSchema, objectType } from '../../src'
import { SchemaConfig } from '../../src/core'
import { ConnectionFieldConfig, ConnectionPluginConfig } from '../../src/plugins/connectionPlugin'

const userNodes: { id: string; name: string }[] = []
for (let i = 0; i < 10; i++) {
userNodes.push({ id: `User:${i + 1}`, name: `Test ${i + 1}` })
}

const User = objectType({
name: 'User',
definition(t) {
t.id('id')
t.string('name')
},
})

const UsersFieldBody = `
nodes { id }
edges {
cursor
node { id }
}
pageInfo {
hasNextPage
hasPreviousPage
startCursor
endCursor
}
`

const UsersFirst = parse(`query UsersFieldFirst($first: Int!) { users(first: $first) { ${UsersFieldBody} } }`)

const makeTestSchema = (
pluginConfig: ConnectionPluginConfig = {},
fieldConfig: Omit<ConnectionFieldConfig<any, any>, 'type'> = {},
makeSchemaConfig: Omit<SchemaConfig, 'types'> = {}
) =>
makeSchema({
outputs: false,
types: [
User,
objectType({
name: 'Query',
definition(t) {
// @ts-ignore
t.connectionField('users', {
type: User,
nodes(root: any, args: any, ctx: any, info: any) {
return userNodes
},
...fieldConfig,
})
},
}),
],
nonNullDefaults: {
input: false,
output: false,
},
...makeSchemaConfig,
plugins: [connectionPlugin(pluginConfig), ...(makeSchemaConfig.plugins ?? [])],
})

beforeEach(() => {
jest.resetAllMocks()
})

describe('field level configuration', () => {
it('does not pay the cost of async unless there is an async call', () => {
const schema = makeTestSchema()

const result = executeSync({
schema,
document: UsersFirst,
variableValues: {
first: 1,
},
})
expect(result).toMatchSnapshot()
})
})
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5814,10 +5814,10 @@ prettier@^1.16.0:
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.19.1.tgz#f7d7f5ff8a9cd872a7be4ca142095956a60797cb"
integrity sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew==

prettier@^2.0.5:
version "2.0.5"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.0.5.tgz#d6d56282455243f2f92cc1716692c08aa31522d4"
integrity sha512-7PtVymN48hGcO4fGjybyBSIWDsLU4H4XlvOHfq91pz9kkGlonzwTfYkaIEwiRg/dAJF9YlbsduBAgtYLi+8cFg==
prettier@^2.2.1:
version "2.2.1"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.2.1.tgz#795a1a78dd52f073da0cd42b21f9c91381923ff5"
integrity sha512-PqyhM2yCjg/oKkFPtTGUojv7gnZAoG80ttl45O6x2Ug/rMJw4wcc9k6aaf2hibP7BGVCCM33gZoGjyvt9mm16Q==

pretty-format@^23.6.0:
version "23.6.0"
Expand Down

0 comments on commit 0cff220

Please sign in to comment.