Skip to content

Commit f786f49

Browse files
glasserglasser
committed
Experiment with simplifying new defer/stream execute types
Use distinct types for "the only result of a single-payload execution", "the first payload of a multi-payload execution", and "subsequent payloads of a multi-payload execution", since the data structures are different. Wrap the first payload and the subsequent payloads in an `AsyncExecutionResultStream`. You can now differentiate this from the single-result case by checking `'initialResult' in result` rather than `isAsyncIterable`; TS inference works better this way. flattenAsyncIterable now works on iterables of precisely one nesting level, and it fixes graphql#3709.
1 parent 172c853 commit f786f49

File tree

12 files changed

+326
-276
lines changed

12 files changed

+326
-276
lines changed

‎src/execution/__tests__/defer-test.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { describe, it } from 'mocha';
33
import { expectJSON } from '../../__testUtils__/expectJSON';
44
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick';
55

6-
import { isAsyncIterable } from '../../jsutils/isAsyncIterable';
7-
86
import type { DocumentNode } from '../../language/ast';
97
import { parse } from '../../language/parser';
108

@@ -16,6 +14,10 @@ import {
1614
import { GraphQLID, GraphQLString } from '../../type/scalars';
1715
import { GraphQLSchema } from '../../type/schema';
1816

17+
import type {
18+
InitialAsyncExecutionResult,
19+
SubsequentAsyncExecutionResult,
20+
} from '../execute';
1921
import { execute } from '../execute';
2022

2123
const friendType = new GraphQLObjectType({
@@ -86,9 +88,11 @@ async function complete(document: DocumentNode) {
8688
rootValue: {},
8789
});
8890

89-
if (isAsyncIterable(result)) {
90-
const results = [];
91-
for await (const patch of result) {
91+
if ('initialResult' in result) {
92+
const results: Array<
93+
InitialAsyncExecutionResult | SubsequentAsyncExecutionResult
94+
> = [result.initialResult];
95+
for await (const patch of result.subsequentResults) {
9296
results.push(patch);
9397
}
9498
return results;

‎src/execution/__tests__/flattenAsyncIterable-test.ts‎

Lines changed: 6 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -4,153 +4,20 @@ import { describe, it } from 'mocha';
44
import { flattenAsyncIterable } from '../flattenAsyncIterable';
55

66
describe('flattenAsyncIterable', () => {
7-
it('does not modify an already flat async generator', async () => {
8-
async function* source() {
9-
yield await Promise.resolve(1);
10-
yield await Promise.resolve(2);
11-
yield await Promise.resolve(3);
12-
}
13-
14-
const result = flattenAsyncIterable(source());
15-
16-
expect(await result.next()).to.deep.equal({ value: 1, done: false });
17-
expect(await result.next()).to.deep.equal({ value: 2, done: false });
18-
expect(await result.next()).to.deep.equal({ value: 3, done: false });
19-
expect(await result.next()).to.deep.equal({
20-
value: undefined,
21-
done: true,
22-
});
23-
});
24-
25-
it('does not modify an already flat async iterator', async () => {
26-
const items = [1, 2, 3];
27-
28-
const iterator: any = {
29-
[Symbol.asyncIterator]() {
30-
return this;
31-
},
32-
next() {
33-
return Promise.resolve({
34-
done: items.length === 0,
35-
value: items.shift(),
36-
});
37-
},
38-
};
39-
40-
const result = flattenAsyncIterable(iterator);
41-
42-
expect(await result.next()).to.deep.equal({ value: 1, done: false });
43-
expect(await result.next()).to.deep.equal({ value: 2, done: false });
44-
expect(await result.next()).to.deep.equal({ value: 3, done: false });
45-
expect(await result.next()).to.deep.equal({
46-
value: undefined,
47-
done: true,
48-
});
49-
});
50-
51-
it('flatten nested async generators', async () => {
52-
async function* source() {
53-
yield await Promise.resolve(1);
54-
yield await Promise.resolve(2);
55-
yield await Promise.resolve(
56-
(async function* nested(): AsyncGenerator<number, void, void> {
57-
yield await Promise.resolve(2.1);
58-
yield await Promise.resolve(2.2);
59-
})(),
60-
);
61-
yield await Promise.resolve(3);
62-
}
63-
64-
const doubles = flattenAsyncIterable(source());
65-
66-
const result = [];
67-
for await (const x of doubles) {
68-
result.push(x);
69-
}
70-
expect(result).to.deep.equal([1, 2, 2.1, 2.2, 3]);
71-
});
72-
73-
it('allows returning early from a nested async generator', async () => {
7+
it('completely yields sub-iterables even when next() called in parallel', async () => {
748
async function* source() {
75-
yield await Promise.resolve(1);
76-
yield await Promise.resolve(2);
779
yield await Promise.resolve(
7810
(async function* nested(): AsyncGenerator<number, void, void> {
79-
yield await Promise.resolve(2.1); /* c8 ignore start */
80-
// Not reachable, early return
81-
yield await Promise.resolve(2.2);
11+
yield await Promise.resolve(1.1);
12+
yield await Promise.resolve(1.2);
8213
})(),
8314
);
84-
// Not reachable, early return
85-
yield await Promise.resolve(3);
86-
}
87-
/* c8 ignore stop */
88-
89-
const doubles = flattenAsyncIterable(source());
90-
91-
expect(await doubles.next()).to.deep.equal({ value: 1, done: false });
92-
expect(await doubles.next()).to.deep.equal({ value: 2, done: false });
93-
expect(await doubles.next()).to.deep.equal({ value: 2.1, done: false });
94-
95-
// Early return
96-
expect(await doubles.return()).to.deep.equal({
97-
value: undefined,
98-
done: true,
99-
});
100-
101-
// Subsequent next calls
102-
expect(await doubles.next()).to.deep.equal({
103-
value: undefined,
104-
done: true,
105-
});
106-
expect(await doubles.next()).to.deep.equal({
107-
value: undefined,
108-
done: true,
109-
});
110-
});
111-
112-
it('allows throwing errors from a nested async generator', async () => {
113-
async function* source() {
114-
yield await Promise.resolve(1);
115-
yield await Promise.resolve(2);
11615
yield await Promise.resolve(
11716
(async function* nested(): AsyncGenerator<number, void, void> {
118-
yield await Promise.resolve(2.1); /* c8 ignore start */
119-
// Not reachable, early return
17+
yield await Promise.resolve(2.1);
12018
yield await Promise.resolve(2.2);
12119
})(),
12220
);
123-
// Not reachable, early return
124-
yield await Promise.resolve(3);
125-
}
126-
/* c8 ignore stop */
127-
128-
const doubles = flattenAsyncIterable(source());
129-
130-
expect(await doubles.next()).to.deep.equal({ value: 1, done: false });
131-
expect(await doubles.next()).to.deep.equal({ value: 2, done: false });
132-
expect(await doubles.next()).to.deep.equal({ value: 2.1, done: false });
133-
134-
// Throw error
135-
let caughtError;
136-
try {
137-
await doubles.throw('ouch'); /* c8 ignore start */
138-
// Not reachable, always throws
139-
/* c8 ignore stop */
140-
} catch (e) {
141-
caughtError = e;
142-
}
143-
expect(caughtError).to.equal('ouch');
144-
});
145-
it('completely yields sub-iterables even when next() called in parallel', async () => {
146-
async function* source() {
147-
yield await Promise.resolve(
148-
(async function* nested(): AsyncGenerator<number, void, void> {
149-
yield await Promise.resolve(1.1);
150-
yield await Promise.resolve(1.2);
151-
})(),
152-
);
153-
yield await Promise.resolve(2);
15421
}
15522

15623
const result = flattenAsyncIterable(source());
@@ -159,7 +26,8 @@ describe('flattenAsyncIterable', () => {
15926
const promise2 = result.next();
16027
expect(await promise1).to.deep.equal({ value: 1.1, done: false });
16128
expect(await promise2).to.deep.equal({ value: 1.2, done: false });
162-
expect(await result.next()).to.deep.equal({ value: 2, done: false });
29+
expect(await result.next()).to.deep.equal({ value: 2.1, done: false });
30+
expect(await result.next()).to.deep.equal({ value: 2.2, done: false });
16331
expect(await result.next()).to.deep.equal({
16432
value: undefined,
16533
done: true,

‎src/execution/__tests__/lists-test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { GraphQLSchema } from '../../type/schema';
1414

1515
import { buildSchema } from '../../utilities/buildASTSchema';
1616

17-
import type { AsyncExecutionResult, ExecutionResult } from '../execute';
17+
import type { AsyncExecutionResultStream, ExecutionResult } from '../execute';
1818
import { execute, executeSync } from '../execute';
1919

2020
describe('Execute: Accepts any iterable as list value', () => {
@@ -85,9 +85,7 @@ describe('Execute: Accepts async iterables as list value', () => {
8585

8686
function completeObjectList(
8787
resolve: GraphQLFieldResolver<{ index: number }, unknown>,
88-
): PromiseOrValue<
89-
ExecutionResult | AsyncGenerator<AsyncExecutionResult, void, void>
90-
> {
88+
): PromiseOrValue<ExecutionResult | AsyncExecutionResultStream> {
9189
const schema = new GraphQLSchema({
9290
query: new GraphQLObjectType({
9391
name: 'Query',

‎src/execution/__tests__/mutations-test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import { describe, it } from 'mocha';
44
import { expectJSON } from '../../__testUtils__/expectJSON';
55
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick';
66

7-
import { isAsyncIterable } from '../../jsutils/isAsyncIterable';
8-
97
import { parse } from '../../language/parser';
108

119
import { GraphQLObjectType } from '../../type/definition';
@@ -223,8 +221,9 @@ describe('Execute: Handles mutation execution ordering', () => {
223221
});
224222
const patches = [];
225223

226-
assert(isAsyncIterable(mutationResult));
227-
for await (const patch of mutationResult) {
224+
assert('initialResult' in mutationResult);
225+
patches.push(mutationResult.initialResult);
226+
for await (const patch of mutationResult.subsequentResults) {
228227
patches.push(patch);
229228
}
230229

@@ -298,8 +297,9 @@ describe('Execute: Handles mutation execution ordering', () => {
298297
});
299298
const patches = [];
300299

301-
assert(isAsyncIterable(mutationResult));
302-
for await (const patch of mutationResult) {
300+
assert('initialResult' in mutationResult);
301+
patches.push(mutationResult.initialResult);
302+
for await (const patch of mutationResult.subsequentResults) {
303303
patches.push(patch);
304304
}
305305

‎src/execution/__tests__/nonnull-test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { GraphQLSchema } from '../../type/schema';
1313

1414
import { buildSchema } from '../../utilities/buildASTSchema';
1515

16-
import type { AsyncExecutionResult, ExecutionResult } from '../execute';
16+
import type { AsyncExecutionResultStream, ExecutionResult } from '../execute';
1717
import { execute, executeSync } from '../execute';
1818

1919
const syncError = new Error('sync');
@@ -111,9 +111,7 @@ const schema = buildSchema(`
111111
function executeQuery(
112112
query: string,
113113
rootValue: unknown,
114-
): PromiseOrValue<
115-
ExecutionResult | AsyncGenerator<AsyncExecutionResult, void, void>
116-
> {
114+
): PromiseOrValue<ExecutionResult | AsyncExecutionResultStream> {
117115
return execute({ schema, document: parse(query), rootValue });
118116
}
119117

‎src/execution/__tests__/simplePubSub.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export class SimplePubSub<T> {
5151
emptyQueue();
5252
return Promise.resolve({ value: undefined, done: true });
5353
},
54+
/* c8 ignore next 4 */
5455
throw(error: unknown) {
5556
emptyQueue();
5657
return Promise.reject(error);

0 commit comments

Comments
 (0)