From 1ce4b1efdef732d2635b657df6273a975a19d217 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 2 Jul 2024 22:30:30 +0300 Subject: [PATCH] changes from review --- .../delegate/src/defaultMergedResolver.ts | 4 ++-- packages/delegate/src/leftOver.ts | 17 +--------------- .../src/execution/IncrementalGraph.ts | 5 ++--- .../execution/__tests__/abort-signal.test.ts | 5 ++--- .../src/execution/__tests__/defer-test.ts | 4 ++-- .../src/execution/__tests__/stream-test.ts | 13 ++++++------ .../src/execution/promiseWithResolvers.ts | 20 ------------------- packages/federation/test/defer.test.ts | 1 - packages/utils/src/createDeferred.ts | 16 +++++++++++++++ packages/utils/src/index.ts | 1 + 10 files changed, 32 insertions(+), 54 deletions(-) delete mode 100644 packages/executor/src/execution/promiseWithResolvers.ts create mode 100644 packages/utils/src/createDeferred.ts diff --git a/packages/delegate/src/defaultMergedResolver.ts b/packages/delegate/src/defaultMergedResolver.ts index 4946f6d8ccc..95b2c017ff7 100644 --- a/packages/delegate/src/defaultMergedResolver.ts +++ b/packages/delegate/src/defaultMergedResolver.ts @@ -5,8 +5,8 @@ import { responsePathAsArray, SelectionSetNode, } from 'graphql'; -import { getResponseKeyFromInfo, isPromise } from '@graphql-tools/utils'; -import { createDeferred, DelegationPlanLeftOver, getPlanLeftOverFromParent } from './leftOver.js'; +import { createDeferred, getResponseKeyFromInfo, isPromise } from '@graphql-tools/utils'; +import { DelegationPlanLeftOver, getPlanLeftOverFromParent } from './leftOver.js'; import { getSubschema, getUnpathedErrors, diff --git a/packages/delegate/src/leftOver.ts b/packages/delegate/src/leftOver.ts index a2c6f39aa64..0aa85815d58 100644 --- a/packages/delegate/src/leftOver.ts +++ b/packages/delegate/src/leftOver.ts @@ -1,23 +1,8 @@ import { FieldNode } from 'graphql'; +import { Deferred } from '@graphql-tools/utils'; import { Subschema } from './Subschema.js'; import { DelegationPlanBuilder, ExternalObject } from './types.js'; -export type Deferred = PromiseWithResolvers; - -// TODO: Remove this after Node 22 -export function createDeferred(): Deferred { - if (Promise.withResolvers) { - return Promise.withResolvers(); - } - let resolve: (value: T | PromiseLike) => void; - let reject: (error: unknown) => void; - const promise = new Promise((_resolve, _reject) => { - resolve = _resolve; - reject = _reject; - }); - return { promise, resolve: resolve!, reject: reject! }; -} - export interface DelegationPlanLeftOver { unproxiableFieldNodes: Array; nonProxiableSubschemas: Array; diff --git a/packages/executor/src/execution/IncrementalGraph.ts b/packages/executor/src/execution/IncrementalGraph.ts index 0a4dc5a5632..455ec5b27e2 100644 --- a/packages/executor/src/execution/IncrementalGraph.ts +++ b/packages/executor/src/execution/IncrementalGraph.ts @@ -1,8 +1,7 @@ import type { GraphQLError } from 'graphql'; -import { isPromise } from '@graphql-tools/utils'; +import { createDeferred, isPromise } from '@graphql-tools/utils'; import { BoxedPromiseOrValue } from './BoxedPromiseOrValue.js'; import { invariant } from './invariant.js'; -import { promiseWithResolvers } from './promiseWithResolvers.js'; import type { DeferredFragmentRecord, DeferredGroupedFieldSetRecord, @@ -75,7 +74,7 @@ export class IncrementalGraph { }); } const { promise, resolve } = - promiseWithResolvers>>(); + createDeferred>>(); this._nextQueue.push(resolve); return promise; }, diff --git a/packages/executor/src/execution/__tests__/abort-signal.test.ts b/packages/executor/src/execution/__tests__/abort-signal.test.ts index cee8391aea9..ce6e66bea0b 100644 --- a/packages/executor/src/execution/__tests__/abort-signal.test.ts +++ b/packages/executor/src/execution/__tests__/abort-signal.test.ts @@ -1,7 +1,6 @@ import { parse } from 'graphql'; -import { createDeferred } from '@graphql-tools/delegate'; import { makeExecutableSchema } from '@graphql-tools/schema'; -import { isAsyncIterable } from '@graphql-tools/utils'; +import { createDeferred, isAsyncIterable } from '@graphql-tools/utils'; import { Repeater } from '@repeaterjs/repeater'; import { assertAsyncIterable } from '../../../../loaders/url/tests/test-utils'; import { normalizedExecutor } from '../normalizedExecutor'; @@ -456,7 +455,7 @@ describe('Abort Signal', () => { expect(bResolverGotInvoked).toBe(false); }); it('stops pending stream execution for never-returning incremental delivery (@defer)', async () => { - const aResolverGotInvokedD = createDeferred(); + const aResolverGotInvokedD = createDeferred(); const requestGotCancelledD = createDeferred(); let bResolverGotInvoked = false; diff --git a/packages/executor/src/execution/__tests__/defer-test.ts b/packages/executor/src/execution/__tests__/defer-test.ts index 104e0c42ba4..7dbd281ab19 100644 --- a/packages/executor/src/execution/__tests__/defer-test.ts +++ b/packages/executor/src/execution/__tests__/defer-test.ts @@ -8,10 +8,10 @@ import { GraphQLString, parse, } from 'graphql'; +import { createDeferred } from '@graphql-tools/utils'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js'; import { execute } from '../execute.js'; -import { promiseWithResolvers } from '../promiseWithResolvers.js'; import type { InitialIncrementalExecutionResult, SubsequentIncrementalExecutionResult, @@ -875,7 +875,7 @@ describe('Execute: defer directive', () => { } `); - const { promise: slowFieldPromise, resolve: resolveSlowField } = promiseWithResolvers(); + const { promise: slowFieldPromise, resolve: resolveSlowField } = createDeferred(); let cResolverCalled = false; let eResolverCalled = false; const executeResult = execute({ diff --git a/packages/executor/src/execution/__tests__/stream-test.ts b/packages/executor/src/execution/__tests__/stream-test.ts index 51acf8f323c..38514c65f42 100644 --- a/packages/executor/src/execution/__tests__/stream-test.ts +++ b/packages/executor/src/execution/__tests__/stream-test.ts @@ -8,11 +8,10 @@ import { GraphQLString, parse, } from 'graphql'; -import { MaybePromise } from '@graphql-tools/utils'; +import { createDeferred, MaybePromise } from '@graphql-tools/utils'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js'; import { execute } from '../execute.js'; -import { promiseWithResolvers } from '../promiseWithResolvers.js'; import type { InitialIncrementalExecutionResult, SubsequentIncrementalExecutionResult, @@ -1974,7 +1973,7 @@ describe('Execute: stream directive', () => { ]); }); it('Returns payloads in correct order when parent deferred fragment resolves slower than stream', async () => { - const { promise: slowFieldPromise, resolve: resolveSlowField } = promiseWithResolvers(); + const { promise: slowFieldPromise, resolve: resolveSlowField } = createDeferred(); const document = parse(/* GraphQL */ ` query { nestedObject { @@ -2077,9 +2076,9 @@ describe('Execute: stream directive', () => { }); }); it('Can @defer fields that are resolved after async iterable is complete', async () => { - const { promise: slowFieldPromise, resolve: resolveSlowField } = promiseWithResolvers(); + const { promise: slowFieldPromise, resolve: resolveSlowField } = createDeferred(); const { promise: iterableCompletionPromise, resolve: resolveIterableCompletion } = - promiseWithResolvers(); + createDeferred(); const document = parse(/* GraphQL */ ` query { @@ -2187,9 +2186,9 @@ describe('Execute: stream directive', () => { }); }); it('Can @defer fields that are resolved before async iterable is complete', async () => { - const { promise: slowFieldPromise, resolve: resolveSlowField } = promiseWithResolvers(); + const { promise: slowFieldPromise, resolve: resolveSlowField } = createDeferred(); const { promise: iterableCompletionPromise, resolve: resolveIterableCompletion } = - promiseWithResolvers(); + createDeferred(); const document = parse(/* GraphQL */ ` query { diff --git a/packages/executor/src/execution/promiseWithResolvers.ts b/packages/executor/src/execution/promiseWithResolvers.ts deleted file mode 100644 index eb533e7b4cf..00000000000 --- a/packages/executor/src/execution/promiseWithResolvers.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { MaybePromise } from '@graphql-tools/utils'; - -/** - * Based on Promise.withResolvers proposal - * https://github.com/tc39/proposal-promise-with-resolvers - */ -export function promiseWithResolvers(): { - promise: Promise; - resolve: (value: T | MaybePromise) => void; - reject: (reason?: any) => void; -} { - // these are assigned synchronously within the Promise constructor - let resolve!: (value: T | MaybePromise) => void; - let reject!: (reason?: any) => void; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - return { promise, resolve, reject }; -} diff --git a/packages/federation/test/defer.test.ts b/packages/federation/test/defer.test.ts index 1d47a924a88..5a5fa66ff63 100644 --- a/packages/federation/test/defer.test.ts +++ b/packages/federation/test/defer.test.ts @@ -1,4 +1,3 @@ -import { exec } from 'child_process'; import { inspect } from 'util'; import { GraphQLSchema, parse, print } from 'graphql'; import _ from 'lodash'; diff --git a/packages/utils/src/createDeferred.ts b/packages/utils/src/createDeferred.ts new file mode 100644 index 00000000000..417067a3ab0 --- /dev/null +++ b/packages/utils/src/createDeferred.ts @@ -0,0 +1,16 @@ +// TODO: Remove this after Node 22 + +export type Deferred = PromiseWithResolvers; + +export function createDeferred(): Deferred { + if (Promise.withResolvers) { + return Promise.withResolvers(); + } + let resolve: (value: T | PromiseLike) => void; + let reject: (error: unknown) => void; + const promise = new Promise((_resolve, _reject) => { + resolve = _resolve; + reject = _reject; + }); + return { promise, resolve: resolve!, reject: reject! }; +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index c2bd2e5ab05..3079a72efaa 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -54,3 +54,4 @@ export * from './jsutils.js'; export * from './directives.js'; export * from './mergeIncrementalResult.js'; export * from './debugTimer.js'; +export * from './createDeferred.js';