Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement correct merging of incremental respones (@defer/@stream) #3580

Merged
merged 3 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/clean-pets-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphiql/react': minor
---

Implement correct merging of incremental responses (@defer/@stream)
102 changes: 66 additions & 36 deletions packages/graphiql-react/src/execution.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import {
Fetcher,
FetcherResultPayload,
formatError,
formatResult,
isAsyncIterable,
isObservable,
Unsubscribable,
} from '@graphiql/toolkit';
import { ExecutionResult, FragmentDefinitionNode, print } from 'graphql';
import {
ExecutionResult,
FragmentDefinitionNode,
GraphQLError,
print,
} from 'graphql';
import { getFragmentDependenciesForAST } from 'graphql-language-service';
import { ReactNode, useCallback, useMemo, useRef, useState } from 'react';
import setValue from 'set-value';
Expand Down Expand Up @@ -183,7 +187,7 @@
});

try {
let fullResponse: FetcherResultPayload = { data: {} };
const fullResponse: ExecutionResult = {};

Check warning on line 190 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L190

Added line #L190 was not covered by tests
const handleResponse = (result: ExecutionResult) => {
// A different query was dispatched in the meantime, so don't
// show the results of this one.
Expand All @@ -202,40 +206,8 @@
}

if (maybeMultipart) {
const payload: FetcherResultPayload = {
data: fullResponse.data,
};
const maybeErrors = [
...(fullResponse?.errors || []),
...maybeMultipart.flatMap(i => i.errors).filter(Boolean),
];

if (maybeErrors.length) {
payload.errors = maybeErrors;
}

for (const part of maybeMultipart) {
// We pull out errors here, so we dont include it later
const { path, data, errors, ...rest } = part;
if (path) {
if (!data) {
throw new Error(
`Expected part to contain a data property, but got ${part}`,
);
}

setValue(payload.data, path, data, { merge: true });
} else if (data) {
// If there is no path, we don't know what to do with the payload,
// so we just set it.
payload.data = data;
}

// Ensures we also bring extensions and alike along for the ride
fullResponse = {
...payload,
...rest,
};
mergeIncrementalResult(fullResponse, part);

Check warning on line 210 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L210

Added line #L210 was not covered by tests
}

setIsFetching(false);
Expand Down Expand Up @@ -361,3 +333,61 @@
}
return parsed;
}

type IncrementalResult = {
data?: Record<string, unknown> | null;
errors?: ReadonlyArray<GraphQLError>;
extensions?: Record<string, unknown>;
hasNext?: boolean;
path?: ReadonlyArray<string | number>;
incremental?: ReadonlyArray<IncrementalResult>;
label?: string;
items?: ReadonlyArray<Record<string, unknown>> | null;
};

/**
* @param executionResult The complete execution result object which will be
* mutated by merging the contents of the incremental result.
* @param incrementalResult The incremental result that will be merged into the
* complete execution result.
*/
function mergeIncrementalResult(
executionResult: ExecutionResult,
incrementalResult: IncrementalResult,
): void {
const path = ['data', ...(incrementalResult.path ?? [])];

if (incrementalResult.items) {
for (const item of incrementalResult.items) {
setValue(executionResult, path.join('.'), item);

Check warning on line 362 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L361-L362

Added lines #L361 - L362 were not covered by tests
// Increment the last path segment (the array index) to merge the next item at the next index
// eslint-disable-next-line unicorn/prefer-at -- cannot mutate the array using Array.at()
(path[path.length - 1] as number)++;

Check warning on line 365 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L365

Added line #L365 was not covered by tests
}
}

if (incrementalResult.data) {
setValue(executionResult, path.join('.'), incrementalResult.data, {

Check warning on line 370 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L370

Added line #L370 was not covered by tests
merge: true,
});
}

if (incrementalResult.errors) {
executionResult.errors ||= [];
(executionResult.errors as GraphQLError[]).push(

Check warning on line 377 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L376-L377

Added lines #L376 - L377 were not covered by tests
...incrementalResult.errors,
);
}

if (incrementalResult.extensions) {
setValue(executionResult, 'extensions', incrementalResult.extensions, {

Check warning on line 383 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L383

Added line #L383 was not covered by tests
merge: true,
});
}

if (incrementalResult.incremental) {
for (const incrementalSubResult of incrementalResult.incremental) {
mergeIncrementalResult(executionResult, incrementalSubResult);

Check warning on line 390 in packages/graphiql-react/src/execution.tsx

View check run for this annotation

Codecov / codecov/patch

packages/graphiql-react/src/execution.tsx#L389-L390

Added lines #L389 - L390 were not covered by tests
}
}
}