Skip to content

Commit

Permalink
use undefined for empty
Browse files Browse the repository at this point in the history
  • Loading branch information
@yaacovCR
yaacovCR committed Apr 8, 2024
1 parent 50b3fef commit 8ff203d
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 45 deletions.
38 changes: 23 additions & 15 deletions src/execution/IncrementalPublisher.ts
View file
Expand Up @@ -174,7 +174,7 @@ export interface FormattedCompletedResult {
export function buildIncrementalResponse(
context: IncrementalPublisherContext,
result: ObjMap<unknown>,
errors: ReadonlyArray<GraphQLError>,
errors: ReadonlyArray<GraphQLError> | undefined,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
): ExperimentalIncrementalExecutionResults {
const incrementalPublisher = new IncrementalPublisher(context);
Expand All @@ -186,7 +186,7 @@ export function buildIncrementalResponse(
}

interface IncrementalPublisherContext {
cancellableStreams: Set<StreamRecord>;
cancellableStreams?: Set<StreamRecord> | undefined;
}

/**
Expand Down Expand Up @@ -220,7 +220,7 @@ class IncrementalPublisher {

buildResponse(
data: ObjMap<unknown>,
errors: ReadonlyArray<GraphQLError>,
errors: ReadonlyArray<GraphQLError> | undefined,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
): ExperimentalIncrementalExecutionResults {
this._addIncrementalDataRecords(incrementalDataRecords);
Expand All @@ -229,7 +229,7 @@ class IncrementalPublisher {
const pending = this._pendingSourcesToResults();

const initialResult: InitialIncrementalExecutionResult =
errors.length === 0
errors === undefined
? { data, pending, hasNext: true }
: { errors, data, pending, hasNext: true };

Expand Down Expand Up @@ -441,8 +441,12 @@ class IncrementalPublisher {
};

const returnStreamIterators = async (): Promise<void> => {
const cancellableStreams = this._context.cancellableStreams;
if (cancellableStreams === undefined) {
return;
}
const promises: Array<Promise<unknown>> = [];
for (const streamRecord of this._context.cancellableStreams) {
for (const streamRecord of cancellableStreams) {
if (streamRecord.earlyReturn !== undefined) {
promises.push(streamRecord.earlyReturn());
}
Expand Down Expand Up @@ -516,7 +520,7 @@ class IncrementalPublisher {
);
}

if (deferredGroupedFieldSetResult.incrementalDataRecords.length > 0) {
if (deferredGroupedFieldSetResult.incrementalDataRecords !== undefined) {
this._addIncrementalDataRecords(
deferredGroupedFieldSetResult.incrementalDataRecords,
);
Expand Down Expand Up @@ -586,14 +590,20 @@ class IncrementalPublisher {
if (streamItemsResult.result === undefined) {
this._completed.push({ id });
this._pending.delete(streamRecord);
this._context.cancellableStreams.delete(streamRecord);
const cancellableStreams = this._context.cancellableStreams;
if (cancellableStreams !== undefined) {
cancellableStreams.delete(streamRecord);
}
} else if (streamItemsResult.result === null) {
this._completed.push({
id,
errors: streamItemsResult.errors,
});
this._pending.delete(streamRecord);
this._context.cancellableStreams.delete(streamRecord);
const cancellableStreams = this._context.cancellableStreams;
if (cancellableStreams !== undefined) {
cancellableStreams.delete(streamRecord);
}
streamRecord.earlyReturn?.().catch(() => {
/* c8 ignore next 1 */
// ignore error
Expand All @@ -606,7 +616,7 @@ class IncrementalPublisher {

this._incremental.push(incrementalEntry);

if (streamItemsResult.incrementalDataRecords.length > 0) {
if (streamItemsResult.incrementalDataRecords !== undefined) {
this._addIncrementalDataRecords(
streamItemsResult.incrementalDataRecords,
);
Expand Down Expand Up @@ -663,7 +673,7 @@ function isDeferredGroupedFieldSetRecord(
export interface IncrementalContext {
deferUsageSet: DeferUsageSet | undefined;
path: Path | undefined;
errors: Array<GraphQLError>;
errors?: Array<GraphQLError> | undefined;
}

export type DeferredGroupedFieldSetResult =
Expand All @@ -680,7 +690,7 @@ interface ReconcilableDeferredGroupedFieldSetResult {
deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
path: Array<string | number>;
result: BareDeferredGroupedFieldSetResult;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord> | undefined;
sent?: true | undefined;
}

Expand Down Expand Up @@ -718,7 +728,6 @@ export class DeferredGroupedFieldSetRecord {
const incrementalContext: IncrementalContext = {
deferUsageSet,
path,
errors: [],
};

for (const deferredFragmentRecord of deferredFragmentRecords) {
Expand Down Expand Up @@ -786,7 +795,7 @@ interface NonReconcilableStreamItemsResult {
interface NonTerminatingStreamItemsResult {
streamRecord: StreamRecord;
result: BareStreamItemsResult;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord> | undefined;
}

interface TerminatingStreamItemsResult {
Expand Down Expand Up @@ -826,7 +835,6 @@ export class StreamItemsRecord {
const incrementalContext: IncrementalContext = {
deferUsageSet: undefined,
path: itemPath,
errors: [],
};

this._result = executor(incrementalContext);
Expand All @@ -850,7 +858,7 @@ export class StreamItemsRecord {
? {
...result,
incrementalDataRecords:
result.incrementalDataRecords.length === 0
result.incrementalDataRecords === undefined
? [this.nextStreamItems]
: [this.nextStreamItems, ...result.incrementalDataRecords],
}
Expand Down
69 changes: 39 additions & 30 deletions src/execution/execute.ts
View file
Expand Up @@ -141,8 +141,8 @@ export interface ExecutionContext {
fieldResolver: GraphQLFieldResolver<any, any>;
typeResolver: GraphQLTypeResolver<any, any>;
subscribeFieldResolver: GraphQLFieldResolver<any, any>;
errors: Array<GraphQLError>;
cancellableStreams: Set<StreamRecord>;
errors: Array<GraphQLError> | undefined;
cancellableStreams: Set<StreamRecord> | undefined;
}

export interface ExecutionArgs {
Expand All @@ -163,7 +163,7 @@ export interface StreamUsage {
fieldGroup: FieldGroup;
}

type GraphQLResult<T> = [T, ReadonlyArray<IncrementalDataRecord>];
type GraphQLResult<T> = [T, ReadonlyArray<IncrementalDataRecord> | undefined];

const UNEXPECTED_EXPERIMENTAL_DIRECTIVES =
'The provided schema unexpectedly contains experimental directives (@defer or @stream). These directives may only be utilized if experimental execution features are explicitly enabled.';
Expand Down Expand Up @@ -348,28 +348,28 @@ function withNewDeferredGroupedFieldSets(

function appendNewIncrementalDataRecords(
acc: GraphQLResult<unknown>,
newRecords: ReadonlyArray<IncrementalDataRecord>,
newRecords: ReadonlyArray<IncrementalDataRecord> | undefined,
): void {
if (newRecords.length > 0) {
acc[1] = acc[1].length === 0 ? newRecords : [...acc[1], ...newRecords];
if (newRecords !== undefined) {
acc[1] = acc[1] === undefined ? newRecords : [...acc[1], ...newRecords];
}
}

function withError(
errors: Array<GraphQLError>,
errors: Array<GraphQLError> | undefined,
error: GraphQLError,
): ReadonlyArray<GraphQLError> {
return errors.length === 0 ? [error] : [...errors, error];
return errors === undefined ? [error] : [...errors, error];
}

function buildDataResponse(
exeContext: ExecutionContext,
data: ObjMap<unknown>,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord> | undefined,
): ExecutionResult | ExperimentalIncrementalExecutionResults {
const { errors } = exeContext;
if (incrementalDataRecords.length === 0) {
return errors.length > 0 ? { errors, data } : { data };
if (incrementalDataRecords === undefined) {
return errors !== undefined ? { errors, data } : { data };
}

return buildIncrementalResponse(
Expand Down Expand Up @@ -481,8 +481,8 @@ export function buildExecutionContext(
fieldResolver: fieldResolver ?? defaultFieldResolver,
typeResolver: typeResolver ?? defaultTypeResolver,
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
errors: [],
cancellableStreams: new Set(),
errors: undefined,
cancellableStreams: undefined,
};
}

Expand All @@ -493,7 +493,7 @@ function buildPerEventExecutionContext(
return {
...exeContext,
rootValue: payload,
errors: [],
errors: undefined,
};
}

Expand Down Expand Up @@ -579,7 +579,7 @@ function executeFieldsSerially(
appendNewIncrementalDataRecords(acc, result[1]);
return acc;
},
[Object.create(null), []] as GraphQLResult<ObjMap<unknown>>,
[Object.create(null), undefined] as GraphQLResult<ObjMap<unknown>>,
);
}

Expand All @@ -597,7 +597,7 @@ function executeFields(
deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord> | undefined,
): PromiseOrValue<GraphQLResult<ObjMap<unknown>>> {
const results = Object.create(null);
const acc: GraphQLResult<ObjMap<unknown>> = [results, []];
const acc: GraphQLResult<ObjMap<unknown>> = [results, undefined];
let containsPromise = false;

try {
Expand Down Expand Up @@ -739,7 +739,7 @@ function executeField(
path,
incrementalContext,
);
return [null, []];
return [null, undefined];
});
}
return completed;
Expand All @@ -752,7 +752,7 @@ function executeField(
path,
incrementalContext,
);
return [null, []];
return [null, undefined];
}
}

Expand Down Expand Up @@ -801,7 +801,13 @@ function handleFieldError(

// Otherwise, error protection is applied, logging the error and resolving
// a null value for this field if one is encountered.
(incrementalContext ?? exeContext).errors.push(error);
const context = incrementalContext ?? exeContext;
let errors = context.errors;
if (errors === undefined) {
errors = [];
context.errors = errors;
}
errors.push(error);
}

/**
Expand Down Expand Up @@ -863,7 +869,7 @@ function completeValue(

// If result value is null or undefined then return null.
if (result == null) {
return [null, []];
return [null, undefined];
}

// If field type is List, complete each item in the list with the inner type
Expand All @@ -883,7 +889,7 @@ function completeValue(
// If field type is a leaf type, Scalar or Enum, serialize to a valid value,
// returning null if serialization is not possible.
if (isLeafType(returnType)) {
return [completeLeafValue(returnType, result), []];
return [completeLeafValue(returnType, result), undefined];
}

// If field type is an abstract type, Interface or Union, determine the
Expand Down Expand Up @@ -958,7 +964,7 @@ async function completePromisedValue(
path,
incrementalContext,
);
return [null, []];
return [null, undefined];
}
}

Expand Down Expand Up @@ -1050,7 +1056,7 @@ async function completeAsyncIteratorValue(
): Promise<GraphQLResult<ReadonlyArray<unknown>>> {
let containsPromise = false;
const completedResults: Array<unknown> = [];
const acc: GraphQLResult<Array<unknown>> = [completedResults, []];
const acc: GraphQLResult<Array<unknown>> = [completedResults, undefined];
let index = 0;
// eslint-disable-next-line no-constant-condition
while (true) {
Expand Down Expand Up @@ -1127,7 +1133,7 @@ async function completeAsyncIteratorValueWithPossibleStream(
): Promise<GraphQLResult<ReadonlyArray<unknown>>> {
let containsPromise = false;
const completedResults: Array<unknown> = [];
const acc: GraphQLResult<Array<unknown>> = [completedResults, []];
const acc: GraphQLResult<Array<unknown>> = [completedResults, undefined];
let index = 0;
const initialCount = streamUsage.initialCount;
// eslint-disable-next-line no-constant-condition
Expand All @@ -1139,6 +1145,9 @@ async function completeAsyncIteratorValueWithPossibleStream(
earlyReturn: asyncIterator.return?.bind(asyncIterator),
});

if (exeContext.cancellableStreams === undefined) {
exeContext.cancellableStreams = new Set();
}
exeContext.cancellableStreams.add(streamRecord);

const firstStreamItems = firstAsyncStreamItems(
Expand Down Expand Up @@ -1320,7 +1329,7 @@ function completeIterableValue(
// where the list contains no Promises by avoiding creating another Promise.
let containsPromise = false;
const completedResults: Array<unknown> = [];
const acc: GraphQLResult<Array<unknown>> = [completedResults, []];
const acc: GraphQLResult<Array<unknown>> = [completedResults, undefined];
let index = 0;
for (const item of items) {
// No need to modify the info object containing the path,
Expand Down Expand Up @@ -1381,7 +1390,7 @@ function completeIterableValueWithPossibleStream(
// where the list contains no Promises by avoiding creating another Promise.
let containsPromise = false;
const completedResults: Array<unknown> = [];
const acc: GraphQLResult<Array<unknown>> = [completedResults, []];
const acc: GraphQLResult<Array<unknown>> = [completedResults, undefined];
let index = 0;
const initialCount = streamUsage.initialCount;
const iterator = items[Symbol.iterator]();
Expand Down Expand Up @@ -2334,7 +2343,7 @@ function buildDeferredGroupedFieldSetResult(
deferredFragmentRecords,
path: pathToArray(path),
result:
errors.length === 0 ? { data: result[0] } : { data: result[0], errors },
errors === undefined ? { data: result[0] } : { data: result[0], errors },
incrementalDataRecords: result[1],
};
}
Expand Down Expand Up @@ -2570,7 +2579,7 @@ function completeStreamItems(
itemPath,
incrementalContext,
);
result = [null, []];
result = [null, undefined];
}
} catch (error) {
return {
Expand All @@ -2591,7 +2600,7 @@ function completeStreamItems(
itemPath,
incrementalContext,
);
return [null, []] as GraphQLResult<unknown>;
return [null, undefined] as GraphQLResult<unknown>;
})
.then(
(resolvedItem) =>
Expand Down Expand Up @@ -2620,7 +2629,7 @@ function buildStreamItemsResult(
return {
streamRecord,
result:
errors.length === 0
errors === undefined
? { items: [result[0]] }
: {
items: [result[0]],
Expand Down

0 comments on commit 8ff203d

Please sign in to comment.