Skip to content

Commit d1dc85f

Browse files
committed
incrementalDelivery: use single execute function with flag being directives themselves
The use of new experimental functionality triggered by directives does not need a separate runtime flag. The directives themselves are the flags and all operations passed to `execute` are assumed to be valid. Using TS conditional types, when new generic `TMaybeIncremental` parameter is set to true, return type of `execute` will be either an ExecutionResult or an IncrementalExecutionResultMap containing an `initialResult` and a generator of `subsequentResults`. While incremental delivery is designated as experimental, `TMaybeIncremental` will default to `false`. When incremental delivery is merged into the spec, it may be useful to adjust the default to `true`. Note that this PR removes the `ExperimentalExecuteIncrementallyResults`, instead favoring a union of ExecutionResults and IncrementalExecutionResultsMap as above, avoiding the unnecessary runtime wrapping of single results.
1 parent a042ff6 commit d1dc85f

File tree

10 files changed

+74
-255
lines changed

10 files changed

+74
-255
lines changed

src/execution/__tests__/defer-test.ts

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
import { expect } from 'chai';
21
import { describe, it } from 'mocha';
32

43
import { expectJSON } from '../../__testUtils__/expectJSON.js';
5-
import { expectPromise } from '../../__testUtils__/expectPromise.js';
64
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
75

86
import type { DocumentNode } from '../../language/ast.js';
@@ -20,7 +18,7 @@ import type {
2018
InitialIncrementalExecutionResult,
2119
SubsequentIncrementalExecutionResult,
2220
} from '../execute.js';
23-
import { execute, experimentalExecuteIncrementally } from '../execute.js';
21+
import { execute } from '../execute.js';
2422

2523
const friendType = new GraphQLObjectType({
2624
fields: {
@@ -84,7 +82,7 @@ const query = new GraphQLObjectType({
8482
const schema = new GraphQLSchema({ query });
8583

8684
async function complete(document: DocumentNode) {
87-
const result = await experimentalExecuteIncrementally({
85+
const result = await execute<true>({
8886
schema,
8987
document,
9088
rootValue: {},
@@ -656,46 +654,4 @@ describe('Execute: defer directive', () => {
656654
},
657655
]);
658656
});
659-
660-
it('original execute function throws error if anything is deferred and everything else is sync', () => {
661-
const doc = `
662-
query Deferred {
663-
... @defer { hero { id } }
664-
}
665-
`;
666-
expect(() =>
667-
execute({
668-
schema,
669-
document: parse(doc),
670-
rootValue: {},
671-
}),
672-
).to.throw(
673-
'Executing this GraphQL operation would unexpectedly produce multiple payloads (due to @defer or @stream directive)',
674-
);
675-
});
676-
677-
it('original execute function resolves to error if anything is deferred and something else is async', async () => {
678-
const doc = `
679-
query Deferred {
680-
hero { slowField }
681-
... @defer { hero { id } }
682-
}
683-
`;
684-
expectJSON(
685-
await expectPromise(
686-
execute({
687-
schema,
688-
document: parse(doc),
689-
rootValue: {},
690-
}),
691-
).toResolve(),
692-
).toDeepEqual({
693-
errors: [
694-
{
695-
message:
696-
'Executing this GraphQL operation would unexpectedly produce multiple payloads (due to @defer or @stream directive)',
697-
},
698-
],
699-
});
700-
});
701657
});

src/execution/__tests__/executor-test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,12 @@ describe('Execute: Handles basic execution tasks', () => {
807807
const rootValue = { a: 'b', c: 'd' };
808808
const operationName = 'Q';
809809

810-
const result = executeSync({ schema, document, rootValue, operationName });
810+
const result = executeSync({
811+
schema,
812+
document,
813+
rootValue,
814+
operationName,
815+
});
811816
expect(result).to.deep.equal({ data: { a: 'b' } });
812817
});
813818

src/execution/__tests__/mutations-test.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ import { GraphQLObjectType } from '../../type/definition.js';
1010
import { GraphQLInt } from '../../type/scalars.js';
1111
import { GraphQLSchema } from '../../type/schema.js';
1212

13-
import {
14-
execute,
15-
executeSync,
16-
experimentalExecuteIncrementally,
17-
} from '../execute.js';
13+
import { execute, executeSync } from '../execute.js';
1814

1915
class NumberHolder {
2016
theNumber: number;
@@ -218,7 +214,7 @@ describe('Execute: Handles mutation execution ordering', () => {
218214
`);
219215

220216
const rootValue = new Root(6);
221-
const mutationResult = await experimentalExecuteIncrementally({
217+
const mutationResult = await execute<true>({
222218
schema,
223219
document,
224220
rootValue,
@@ -294,7 +290,7 @@ describe('Execute: Handles mutation execution ordering', () => {
294290
`);
295291

296292
const rootValue = new Root(6);
297-
const mutationResult = await experimentalExecuteIncrementally({
293+
const mutationResult = await execute<true>({
298294
schema,
299295
document,
300296
rootValue,

src/execution/__tests__/stream-test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import type {
2020
InitialIncrementalExecutionResult,
2121
SubsequentIncrementalExecutionResult,
2222
} from '../execute.js';
23-
import { experimentalExecuteIncrementally } from '../execute.js';
23+
import { execute } from '../execute.js';
2424

2525
const friendType = new GraphQLObjectType({
2626
fields: {
@@ -69,7 +69,7 @@ const query = new GraphQLObjectType({
6969
const schema = new GraphQLSchema({ query });
7070

7171
async function complete(document: DocumentNode, rootValue: unknown = {}) {
72-
const result = await experimentalExecuteIncrementally({
72+
const result = await execute<true>({
7373
schema,
7474
document,
7575
rootValue,
@@ -92,7 +92,7 @@ async function completeAsync(
9292
numCalls: number,
9393
rootValue: unknown = {},
9494
) {
95-
const result = await experimentalExecuteIncrementally({
95+
const result = await execute<true>({
9696
schema,
9797
document,
9898
rootValue,
@@ -1017,7 +1017,7 @@ describe('Execute: stream directive', () => {
10171017
}
10181018
}
10191019
`);
1020-
const executeResult = await experimentalExecuteIncrementally({
1020+
const executeResult = await execute<true>({
10211021
schema,
10221022
document,
10231023
rootValue: {
@@ -1110,7 +1110,7 @@ describe('Execute: stream directive', () => {
11101110
}
11111111
`);
11121112

1113-
const executeResult = await experimentalExecuteIncrementally({
1113+
const executeResult = await execute<true>({
11141114
schema,
11151115
document,
11161116
rootValue: {
@@ -1196,7 +1196,7 @@ describe('Execute: stream directive', () => {
11961196
}
11971197
`);
11981198

1199-
const executeResult = await experimentalExecuteIncrementally({
1199+
const executeResult = await execute<true>({
12001200
schema,
12011201
document,
12021202
rootValue: {
@@ -1300,7 +1300,7 @@ describe('Execute: stream directive', () => {
13001300
}
13011301
`);
13021302

1303-
const executeResult = await experimentalExecuteIncrementally({
1303+
const executeResult = await execute<true>({
13041304
schema,
13051305
document,
13061306
rootValue: {
@@ -1354,7 +1354,7 @@ describe('Execute: stream directive', () => {
13541354
}
13551355
`);
13561356

1357-
const executeResult = await experimentalExecuteIncrementally({
1357+
const executeResult = await execute<true>({
13581358
schema,
13591359
document,
13601360
rootValue: {
@@ -1414,7 +1414,7 @@ describe('Execute: stream directive', () => {
14141414
}
14151415
`);
14161416

1417-
const executeResult = await experimentalExecuteIncrementally({
1417+
const executeResult = await execute<true>({
14181418
schema,
14191419
document,
14201420
rootValue: {

src/execution/__tests__/subscribe-test.ts

Lines changed: 2 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,7 @@ import {
2121
import { GraphQLSchema } from '../../type/schema.js';
2222

2323
import type { ExecutionArgs, ExecutionResult } from '../execute.js';
24-
import {
25-
createSourceEventStream,
26-
experimentalSubscribeIncrementally,
27-
subscribe,
28-
} from '../execute.js';
24+
import { createSourceEventStream, subscribe } from '../execute.js';
2925

3026
import { SimplePubSub } from './simplePubSub.js';
3127

@@ -99,7 +95,6 @@ const emailSchema = new GraphQLSchema({
9995
function createSubscription(
10096
pubsub: SimplePubSub<Email>,
10197
variableValues?: { readonly [variable: string]: unknown },
102-
originalSubscribe: boolean = false,
10398
) {
10499
const document = parse(`
105100
subscription ($priority: Int = 0, $shouldDefer: Boolean = false, $asyncResolver: Boolean = false) {
@@ -145,7 +140,7 @@ function createSubscription(
145140
}),
146141
};
147142

148-
return (originalSubscribe ? subscribe : experimentalSubscribeIncrementally)({
143+
return subscribe({
149144
schema: emailSchema,
150145
document,
151146
rootValue: data,
@@ -841,75 +836,6 @@ describe('Subscription Publish Phase', () => {
841836
});
842837
});
843838

844-
it('original subscribe function returns errors with @defer', async () => {
845-
const pubsub = new SimplePubSub<Email>();
846-
const subscription = await createSubscription(
847-
pubsub,
848-
{
849-
shouldDefer: true,
850-
},
851-
true,
852-
);
853-
assert(isAsyncIterable(subscription));
854-
// Wait for the next subscription payload.
855-
const payload = subscription.next();
856-
857-
// A new email arrives!
858-
expect(
859-
pubsub.emit({
860-
from: 'yuzhi@graphql.org',
861-
subject: 'Alright',
862-
message: 'Tests are good',
863-
unread: true,
864-
}),
865-
).to.equal(true);
866-
867-
const errorPayload = {
868-
done: false,
869-
value: {
870-
errors: [
871-
{
872-
message:
873-
'Executing this GraphQL operation would unexpectedly produce multiple payloads (due to @defer or @stream directive)',
874-
},
875-
],
876-
},
877-
};
878-
879-
// The previously waited on payload now has a value.
880-
expectJSON(await payload).toDeepEqual(errorPayload);
881-
882-
// Wait for the next payload from @defer
883-
expectJSON(await subscription.next()).toDeepEqual(errorPayload);
884-
885-
// Another new email arrives, after all incrementally delivered payloads are received.
886-
expect(
887-
pubsub.emit({
888-
from: 'hyo@graphql.org',
889-
subject: 'Tools',
890-
message: 'I <3 making things',
891-
unread: true,
892-
}),
893-
).to.equal(true);
894-
895-
// The next waited on payload will have a value.
896-
expectJSON(await subscription.next()).toDeepEqual(errorPayload);
897-
// The next waited on payload will have a value.
898-
expectJSON(await subscription.next()).toDeepEqual(errorPayload);
899-
900-
// The client disconnects before the deferred payload is consumed.
901-
expectJSON(await subscription.return()).toDeepEqual({
902-
done: true,
903-
value: undefined,
904-
});
905-
906-
// Awaiting a subscription after closing it results in completed results.
907-
expectJSON(await subscription.next()).toDeepEqual({
908-
done: true,
909-
value: undefined,
910-
});
911-
});
912-
913839
it('produces a payload when there are multiple events', async () => {
914840
const pubsub = new SimplePubSub<Email>();
915841
const subscription = createSubscription(pubsub);

src/execution/__tests__/sync-test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@ describe('Execute: synchronously when possible', () => {
114114
}).to.throw('GraphQL execution failed to complete synchronously.');
115115
});
116116

117+
it('return successfully if incremental delivery is enabled but async iterable is not returned', () => {
118+
const doc = 'query Example { syncField }';
119+
const result = executeSync({
120+
schema,
121+
document: parse(doc),
122+
rootValue: 'rootValue',
123+
});
124+
expect(result).to.deep.equal({ data: { syncField: 'rootValue' } });
125+
});
126+
117127
it('throws if encountering async iterable execution', () => {
118128
const doc = `
119129
query Example {

0 commit comments

Comments
 (0)