Skip to content

Commit

Permalink
upgrade to graphql v17 including support for new incremental delivery…
Browse files Browse the repository at this point in the history
… format (#3682)

* add support for new incremental delivery format

* fix vitest

* add patch for graphql-js bug

submittes upstream with test at graphql/graphql-js#4160

* apply feedback

* move patches!

* some fixes

* fix netlify/cypress

* remove export

* update changeset

---------

Co-authored-by: Dimitri POSTOLOV <[email protected]>
  • Loading branch information
yaacovCR and dimaMachina authored Aug 13, 2024
1 parent 1c1d2b5 commit 6c9f0df
Show file tree
Hide file tree
Showing 34 changed files with 407 additions and 230 deletions.
19 changes: 19 additions & 0 deletions .changeset/wild-frogs-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'graphql-language-service-server': minor
'@graphiql/plugin-code-exporter': minor
'graphql-language-service-cli': minor
'@graphiql/plugin-explorer': minor
'graphql-language-service': minor
'vscode-graphql-execution': minor
'codemirror-graphql': minor
'@graphiql/toolkit': minor
'@graphiql/react': minor
'monaco-graphql': minor
'vscode-graphql': minor
'cm6-graphql': minor
'graphiql': minor
---

Support v17 of `graphql-js` from `17.0.0-alpha.2` forward.

Includes support for the latest incremental delivery response format. For further details, see https://github.com/graphql/defer-stream-wg/discussions/69.
3 changes: 2 additions & 1 deletion .github/workflows/pr-graphql-compat-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
release: ['15.5.3', '^15.8.0', '16.1.0', '16.2.0', '16.3.0']
release:
['15.5.3', '^15.8.0', '16.1.0', '16.2.0', '16.3.0', '17.0.0-alpha.5']
steps:
- name: Checkout Code
uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion functions/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function createHandler<Context extends OperationContext = undefined>(
statusCode: init.status,
};
} catch (err) {
// The handler should'nt throw errors.
// The handler shouldn't throw errors.
// If you wish to handle them differently, consider implementing your own request handler.
console.error(
'Internal error occurred during request handling. ' +
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"gen-agenda": "wgutils agenda gen"
},
"dependencies": {
"graphql-http": "^1.22.1",
"@babel/cli": "^7.21.0",
"@babel/core": "^7.21.0",
"@babel/plugin-proposal-class-properties": "^7.18.6",
Expand Down Expand Up @@ -143,6 +144,7 @@
"resolutions": {
"@babel/traverse": "^7.23.2",
"vscode-languageserver-types": "3.17.3",
"markdown-it": "14.1.0"
"markdown-it": "14.1.0",
"graphql": "17.0.0-alpha.5"
}
}
4 changes: 2 additions & 2 deletions packages/cm6-graphql/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"@lezer/highlight": "^1.0.0",
"@lezer/lr": "^1.1.0",
"esbuild": "0.18.10",
"graphql": "^16.8.1",
"graphql": "^17.0.0-alpha.5",
"rollup": "^2.60.2",
"rollup-plugin-dts": "^4.0.1",
"rollup-plugin-esbuild": "^4.9.1",
Expand All @@ -44,7 +44,7 @@
"@codemirror/state": "6.2.0",
"@codemirror/view": "6.2.1",
"@lezer/highlight": "^1.0.0",
"graphql": "^16.5.0"
"graphql": "^16.5.0 || ^17.0.0-alpha.2"
},
"license": "MIT"
}
4 changes: 2 additions & 2 deletions packages/codemirror-graphql/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"peerDependencies": {
"@codemirror/language": "6.0.0",
"codemirror": "^5.65.3",
"graphql": "^15.5.0 || ^16.0.0"
"graphql": "^15.5.0 || ^16.0.0 || ^17.0.0-alpha.2"
},
"// TEMPORARILY PINNED until we fix graphql 15 support": "",
"dependencies": {
Expand All @@ -51,7 +51,7 @@
"@codemirror/language": "^6.0.0",
"codemirror": "^5.65.3",
"cross-env": "^7.0.2",
"graphql": "^16.8.1",
"graphql": "^17.0.0-alpha.5",
"rimraf": "^3.0.2",
"sane": "2.0.0"
}
Expand Down
10 changes: 8 additions & 2 deletions packages/codemirror-graphql/src/__tests__/lint-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import CodeMirror from 'codemirror';
import 'codemirror/addon/lint/lint';
import { readFileSync } from 'node:fs';
import { join } from 'node:path';
import { GraphQLError, OperationDefinitionNode } from 'graphql';
import { GraphQLError, OperationDefinitionNode, version } from 'graphql';
import '../lint';
import '../mode';
import { TestSchema } from './testSchema';
Expand Down Expand Up @@ -61,7 +61,13 @@ describe('graphql-lint', () => {
const noMutationOperationRule = (context: any) => ({
OperationDefinition(node: OperationDefinitionNode) {
if (node.operation === 'mutation') {
context.reportError(new GraphQLError('I like turtles.', node));
context.reportError(
new GraphQLError(
'I like turtles.',
// @ts-expect-error
parseInt(version, 10) > 16 ? { nodes: node } : node,
),
);
}
return false;
},
Expand Down
4 changes: 2 additions & 2 deletions packages/graphiql-plugin-code-exporter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@
},
"peerDependencies": {
"@graphiql/react": "^0.23.0",
"graphql": "^15.5.0 || ^16.0.0",
"graphql": "^15.5.0 || ^16.0.0 || ^17.0.0-alpha.2",
"react": "^16.8.0 || ^17 || ^18",
"react-dom": "^16.8.0 || ^17 || ^18"
},
"devDependencies": {
"@graphiql/react": "^0.23.0",
"@vitejs/plugin-react": "^4.3.1",
"graphql": "^16.8.1",
"graphql": "^17.0.0-alpha.5",
"postcss-nesting": "^10.1.7",
"react": "^18.2.0",
"react-dom": "^18.2.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/graphiql-plugin-explorer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@
},
"peerDependencies": {
"@graphiql/react": "^0.23.0",
"graphql": "^15.5.0 || ^16.0.0",
"graphql": "^15.5.0 || ^16.0.0 || ^17.0.0-alpha.2",
"react": "^16.8.0 || ^17 || ^18",
"react-dom": "^16.8.0 || ^17 || ^18"
},
"devDependencies": {
"@graphiql/react": "^0.23.0",
"@vitejs/plugin-react": "^4.3.1",
"graphql": "^16.8.1",
"graphql": "^17.0.0-alpha.5",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"typescript": "^4.6.3",
Expand Down
8 changes: 5 additions & 3 deletions packages/graphiql-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"build": "tsc --emitDeclarationOnly && vite build"
},
"peerDependencies": {
"graphql": "^15.5.0 || ^16.0.0",
"graphql": "^15.5.0 || ^16.0.0 || ^17.0.0-alpha.2",
"react": "^16.8.0 || ^17 || ^18",
"react-dom": "^16.8.0 || ^17 || ^18"
},
Expand All @@ -60,17 +60,19 @@
"codemirror-graphql": "^2.0.13",
"copy-to-clipboard": "^3.2.0",
"framer-motion": "^6.5.1",
"get-value": "^3.0.1",
"graphql-language-service": "^5.2.2",
"markdown-it": "^14.1.0",
"set-value": "^4.1.0"
},
"devDependencies": {
"@types/markdown-it": "^14.1.2",
"@babel/helper-string-parser": "^7.19.4",
"@testing-library/react": "14.0.0",
"@types/markdown-it": "^14.1.2",
"@types/get-value": "^3.0.5",
"@types/set-value": "^4.0.1",
"@vitejs/plugin-react": "^4.3.1",
"graphql": "^16.8.1",
"graphql": "^17.0.0-alpha.5",
"postcss-nesting": "^10.1.7",
"react": "^18.2.0",
"react-dom": "^18.2.0",
Expand Down
89 changes: 78 additions & 11 deletions packages/graphiql-react/src/execution.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { getFragmentDependenciesForAST } from 'graphql-language-service';
import { ReactNode, useCallback, useMemo, useRef, useState } from 'react';
import setValue from 'set-value';
import getValue from 'get-value';

import { useAutoCompleteLeafs, useEditorContext } from './editor';
import { UseAutoCompleteLeafsArgs } from './editor/hooks';
Expand Down Expand Up @@ -343,31 +344,85 @@ type IncrementalResult = {
incremental?: ReadonlyArray<IncrementalResult>;
label?: string;
items?: ReadonlyArray<Record<string, unknown>> | null;
pending?: ReadonlyArray<{ id: string; path: ReadonlyArray<string | number> }>;
completed?: ReadonlyArray<{
id: string;
errors?: ReadonlyArray<GraphQLError>;
}>;
id?: string;
subPath?: ReadonlyArray<string | number>;
};

const pathsMap = new WeakMap<
ExecutionResult,
Map<string, ReadonlyArray<string | number>>
>();

/**
* @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,
executionResult: IncrementalResult,
incrementalResult: IncrementalResult,
): void {
const path = ['data', ...(incrementalResult.path ?? [])];

if (incrementalResult.items) {
for (const item of incrementalResult.items) {
setValue(executionResult, path.join('.'), item);
// 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)++;
let path: ReadonlyArray<string | number> | undefined = [
'data',
...(incrementalResult.path ?? []),
];

for (const result of [executionResult, incrementalResult]) {
if (result.pending) {
let paths = pathsMap.get(executionResult);
if (paths === undefined) {
paths = new Map();
pathsMap.set(executionResult, paths);
}

for (const { id, path: pendingPath } of result.pending) {
paths.set(id, ['data', ...pendingPath]);
}
}
}

if (incrementalResult.data) {
setValue(executionResult, path.join('.'), incrementalResult.data, {
const { items } = incrementalResult;
if (items) {
const { id } = incrementalResult;
if (id) {
path = pathsMap.get(executionResult)?.get(id);
if (path === undefined) {
throw new Error('Invalid incremental delivery format.');
}

const list = getValue(executionResult, path.join('.'));
list.push(...items);
} else {
path = ['data', ...(incrementalResult.path ?? [])];
for (const item of items) {
setValue(executionResult, path.join('.'), item);
// 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)++;
}
}
}

const { data } = incrementalResult;
if (data) {
const { id } = incrementalResult;
if (id) {
path = pathsMap.get(executionResult)?.get(id);
if (path === undefined) {
throw new Error('Invalid incremental delivery format.');
}
const { subPath } = incrementalResult;
if (subPath !== undefined) {
path = [...path, ...subPath];
}
}
setValue(executionResult, path.join('.'), data, {
merge: true,
});
}
Expand All @@ -390,4 +445,16 @@ function mergeIncrementalResult(
mergeIncrementalResult(executionResult, incrementalSubResult);
}
}

if (incrementalResult.completed) {
// Remove tracking and add additional errors
for (const { id, errors } of incrementalResult.completed) {
pathsMap.get(executionResult)?.delete(id);

if (errors) {
executionResult.errors ||= [];
(executionResult.errors as GraphQLError[]).push(...errors);
}
}
}
}
4 changes: 2 additions & 2 deletions packages/graphiql-toolkit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
"meros": "^1.1.4"
},
"devDependencies": {
"graphql": "^16.8.1",
"graphql": "^17.0.0-alpha.5",
"graphql-ws": "^5.5.5",
"isomorphic-fetch": "^3.0.0",
"subscriptions-transport-ws": "0.11.0"
},
"peerDependencies": {
"graphql": "^15.5.0 || ^16.0.0",
"graphql": "^15.5.0 || ^16.0.0 || ^17.0.0-alpha.2",
"graphql-ws": ">= 4.5.0"
},
"peerDependenciesMeta": {
Expand Down
22 changes: 15 additions & 7 deletions packages/graphiql/cypress/e2e/errors.cy.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { version } from 'graphql';
import { GraphQLError, version } from 'graphql';

describe('Errors', () => {
it('Should show an error when the HTTP request fails', () => {
cy.visit('/?http-error=true');
cy.intercept('/graphql', {
statusCode: 502,
body: 'Bad Gateway',
});
cy.visit('/');
cy.assertQueryResult({
errors: [
{
Expand All @@ -21,23 +25,27 @@ describe('Errors', () => {
});

it('Should show an error when introspection fails', () => {
cy.visit('/?graphql-error=true');
cy.intercept('/graphql', {
body: { errors: [new GraphQLError('Something unexpected happened...')] },
});
cy.visit('/');
cy.assertQueryResult({
errors: [{ message: 'Something unexpected happened...' }],
});
});

it('Should show an error when the schema is invalid', () => {
cy.visit('/?bad=true');
cy.intercept('/graphql', { fixture: 'bad-schema.json' });
cy.visit('/');
/**
* We can't use `cy.assertQueryResult` here because the stack contains line
* and column numbers of the `graphiql.min.js` bundle which are not stable.
*/
cy.get('section.result-window').should(element => {
expect(element.get(0).innerText).to.contain(
version.startsWith('16.')
? 'Names must only contain [_a-zA-Z0-9] but \\"<img src=x onerror=alert(document.domain)>\\" does not.'
: 'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but \\"<img src=x onerror=alert(document.domain)>\\" does not.',
version.startsWith('15')
? 'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but \\"<img src=x onerror=alert(document.domain)>\\" does not.'
: 'Names must only contain [_a-zA-Z0-9] but \\"<img src=x onerror=alert(document.domain)>\\" does not.',
);
});
});
Expand Down
Loading

0 comments on commit 6c9f0df

Please sign in to comment.