Skip to content

Commit

Permalink
fix: graphql 15 fixes, feature flags, tests
Browse files Browse the repository at this point in the history
- fix graphql 15 related issues in several packages
- introduce build & tests against graphql@15
- use version driven logic to make tests pass
  • Loading branch information
acao committed Dec 6, 2021
1 parent 848da58 commit 755d0c4
Show file tree
Hide file tree
Showing 20 changed files with 195 additions and 159 deletions.
9 changes: 9 additions & 0 deletions .changeset/small-bats-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'codemirror-graphql': patch
'graphiql': patch
'graphql-language-service-interface': patch
'graphql-language-service-server': patch
'graphql-language-service-utils': patch
---

Fix graphql 15 related issues. Should now build & test interchangeably.
32 changes: 32 additions & 0 deletions .github/workflows/pr-graphql-15-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Build & Test PR w/ GraphQL 15
on:
pull_request:
types: [opened, synchronize]

# TODO: test matrix?

jobs:
build:
name: Build & Test
runs-on: ubuntu-20.04
steps:
- name: Checkout Code
uses: actions/checkout@v2

- name: Use Node
uses: actions/setup-node@v2

- name: Yarn Install
uses: bahmutov/npm-install@v1

- name: Force GraphQL 15 resolution
run: yarn repo:resolve graphql@15

- name: Typescript Build
run: yarn build

- name: Unit Tests
run: yarn test

- name: Cypress
run: yarn e2e
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@
"lint-check": "eslint --print-config .eslintrc.js | eslint-config-prettier-check",
"lint-fix": "yarn eslint --fix",
"postbuild": "yarn workspace codemirror-graphql run postbuild",
"postpublish": "ts-node ./resources/publishCleanup.ts",
"prebuild-bundles": "yarn build-bundles-clean",
"prepublishOnly": "./resources/prepublish.sh",
"pretty": "node resources/pretty.js",
"pretty-check": "node resources/pretty.js --check",
"prepublishOnly": "./scripts/prepublish.sh",
"pretty": "node scripts/pretty.js",
"pretty-check": "node scripts/pretty.js --check",
"release": "yarn build && yarn build-bundles && yarn changeset publish",
"release:canary": "(node scripts/canary-release.js && yarn build && yarn build-bundles && yarn changeset publish --tag canary) || echo Skipping Canary...",
"repo:lint": "manypkg check",
"repo:fix": "manypkg fix",
"repo:resolve": "node scripts/set-resolution.js",
"start-graphiql": "yarn workspace graphiql dev",
"start-monaco": "yarn workspace example-monaco-graphql-webpack start",
"t": "yarn run testonly",
Expand Down Expand Up @@ -111,13 +111,14 @@
"eslint-plugin-prefer-object-spread": "1.2.1",
"eslint-plugin-react": "7.19.0",
"eslint-plugin-react-hooks": "^3.0.0",
"execa": "^6.0.0",
"express": "^4.17.1",
"fetch-mock": "6.5.2",
"husky": "^4.2.3",
"jest": "^27",
"jest-environment-jsdom": "^27",
"jest-environment-node": "^27",
"jest-environment-jsdom-global": "^2.0.2",
"jest-environment-node": "^27",
"js-green-licenses": "3.0.0",
"jsdom": "^18.1.1",
"lint-staged": "^10.1.2",
Expand Down
6 changes: 3 additions & 3 deletions packages/codemirror-graphql/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@
"scripts": {
"build": "yarn build-clean && yarn build-js && yarn build-esm && yarn build-flow .",
"build-js": "yarn tsc",
"build-esm": "cross-env ESM=true yarn tsc --project tsconfig.esm.json && node ../../resources/renameFileExtensions.js './esm/{**,!**/__tests__/}/*.js' . .esm.js",
"build-esm": "cross-env ESM=true yarn tsc --project tsconfig.esm.json && node ../../scripts/renameFileExtensions.js './esm/{**,!**/__tests__/}/*.js' . .esm.js",
"build-clean": "rimraf {mode,hint,info,jump,lint}.{js,esm.js,js.flow,js.map,d.ts,d.ts.map} && rimraf esm results utils variables coverage cm6-legacy __tests__",
"build-flow": "node ../../resources/buildFlow.js",
"build-flow": "node ../../scripts/buildFlow.js",
"watch": "babel --optional runtime resources/watch.js | node",
"test": "jest",
"postbuild": "node ../../resources/renameFileExtensions.js './esm/{**,!**/__tests__/}/*.js' . .esm.js"
"postbuild": "node ../../scripts/renameFileExtensions.js './esm/{**,!**/__tests__/}/*.js' . .esm.js"
},
"peerDependencies": {
"codemirror": "^5.58.2",
Expand Down
22 changes: 10 additions & 12 deletions packages/codemirror-graphql/src/__tests__/hint-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ import {
GraphQLList,
GraphQLNonNull,
GraphQLString,
__Directive,
__EnumValue,
__Field,
__InputValue,
__Schema,
__Type,
} from 'graphql';
Expand Down Expand Up @@ -613,33 +617,27 @@ describe('graphql-hint', () => {
},
{
text: '__Schema',
description:
'A GraphQL Schema defines the capabilities of a GraphQL server. It exposes all available types and directives on the server, as well as the entry points for query, mutation, and subscription operations.',
description: __Schema.description,
},
{
text: '__Type',
description:
'The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum.\n\nDepending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name, description and optional `specifiedByURL`, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible at runtime. List and NonNull types compose other types.',
description: __Type.description,
},
{
text: '__Field',
description:
'Object and Interface types are described by a list of Fields, each of which has a name, potentially a list of arguments, and a return type.',
description: __Field.description,
},
{
text: '__InputValue',
description:
'Arguments provided to Fields or Directives and the input fields of an InputObject are represented as Input Values which describe their type and optionally a default value.',
description: __InputValue.description,
},
{
text: '__EnumValue',
description:
'One possible value for a given Enum. Enum values are unique values, not a placeholder for a string or numeric value. However an Enum value is returned in a JSON response as a string.',
description: __EnumValue.description,
},
{
text: '__Directive',
description:
"A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.\n\nIn some cases, you need to provide options to alter GraphQL's execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives provide this by describing additional information to the executor.",
description: __Directive.description,
},
];
const expectedSuggestions = getExpectedSuggestions(list);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
describe('IncrementalDelivery support via fetcher', () => {
import { version } from 'graphql/version';

let describeOrSkip = describe.skip;

// TODO: disable when defer/stream is merged to graphql
if (version.includes('stream')) {
describeOrSkip = describe;
}

describeOrSkip('IncrementalDelivery support via fetcher', () => {
describe('When operation contains @stream', () => {
const testStreamQuery = /* GraphQL */ `
query StreamQuery($delay: Int) {
Expand Down
17 changes: 5 additions & 12 deletions packages/graphiql/src/components/GraphiQL.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1052,16 +1052,11 @@ export class GraphiQL extends React.Component<GraphiQLProps, GraphiQLState> {
externalFragments.set(def.name.value, def);
});
} else {
visit(
parse(this.props.externalFragments, {
allowLegacyFragmentVariables: true,
}),
{
FragmentDefinition(def) {
externalFragments.set(def.name.value, def);
},
visit(parse(this.props.externalFragments, {}), {
FragmentDefinition(def) {
externalFragments.set(def.name.value, def);
},
);
});
}
const fragmentDependencies = getFragmentDependenciesForAST(
this.state.documentAST!,
Expand Down Expand Up @@ -1338,9 +1333,7 @@ export class GraphiQL extends React.Component<GraphiQLProps, GraphiQLState> {
handlePrettifyQuery = () => {
const editor = this.getQueryEditor();
const editorContent = editor?.getValue() ?? '';
const prettifiedEditorContent = print(
parse(editorContent, { allowLegacyFragmentVariables: true }),
);
const prettifiedEditorContent = print(parse(editorContent));

if (prettifiedEditorContent !== editorContent) {
editor?.setValue(prettifiedEditorContent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@

import { CompletionItem } from 'graphql-language-service-types';

import fs from 'fs';
import fs, { readSync } from 'fs';
import {
buildSchema,
FragmentDefinitionNode,
GraphQLSchema,
parse,
version as graphQLVersion,
} from 'graphql';
import { Position } from 'graphql-language-service-utils';
import path from 'path';
Expand Down Expand Up @@ -76,6 +77,7 @@ const suggestionCommand = {
describe('getAutocompleteSuggestions', () => {
let schema: GraphQLSchema;
beforeEach(async () => {
// graphQLVersion = pkg.version;
const schemaIDL = fs.readFileSync(
path.join(__dirname, '__schema__/StarWarsSchema.graphql'),
'utf8',
Expand Down Expand Up @@ -432,29 +434,28 @@ describe('getAutocompleteSuggestions', () => {
]);
});

const expectedDirectiveSuggestions = [
{ label: 'include' },
{ label: 'skip' },
];
// TODO: remove this once defer and stream are merged to `graphql`
if (graphQLVersion.includes('defer')) {
expectedDirectiveSuggestions.push({ label: 'stream' });
}
expectedDirectiveSuggestions.push({ label: 'test' });

it('provides correct directive suggestions', () => {
expect(testSuggestions('{ test @ }', new Position(0, 8))).toEqual([
{ label: 'include' },
{ label: 'skip' },
{ label: 'stream' },
{ label: 'test' },
]);
expect(testSuggestions('{ test @ }', new Position(0, 8))).toEqual(
expectedDirectiveSuggestions,
);

expect(testSuggestions('{ test @', new Position(0, 8))).toEqual([
{ label: 'include' },
{ label: 'skip' },
{ label: 'stream' },
{ label: 'test' },
]);
expect(testSuggestions('{ test @', new Position(0, 8))).toEqual(
expectedDirectiveSuggestions,
);

expect(
testSuggestions('{ aliasTest: test @ }', new Position(0, 19)),
).toEqual([
{ label: 'include' },
{ label: 'skip' },
{ label: 'stream' },
{ label: 'test' },
]);
).toEqual(expectedDirectiveSuggestions);

expect(testSuggestions('query @', new Position(0, 7))).toEqual([]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,11 @@ export const SuggestionCommand = {
const collectFragmentDefs = (op: string | undefined) => {
const externalFragments: FragmentDefinitionNode[] = [];
if (op) {
visit(
parse(op, {
allowLegacyFragmentVariables: true,
}),
{
FragmentDefinition(def) {
externalFragments.push(def);
},
visit(parse(op), {
FragmentDefinition(def) {
externalFragments.push(def);
},
);
});
}
return externalFragments;
};
Expand Down
4 changes: 1 addition & 3 deletions packages/graphql-language-service-server/src/GraphQLCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,7 @@ export class GraphQLCache implements GraphQLCacheInterface {
// Return an empty array.
let parsedQuery;
try {
parsedQuery = parse(query, {
allowLegacyFragmentVariables: true,
});
parsedQuery = parse(query);
} catch (error) {
return [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import {
GraphQLInputField,
GraphQLInputType,
GraphQLList,
isEnumType,
isInputObjectType,
isListType,
Expand Down Expand Up @@ -72,8 +71,9 @@ function renderType(into: string[], t: GraphQLInputType | GraphQLInputField) {
if (isNonNullType(t)) {
renderType(into, t.ofType);
text(into, '!');
} else if (t instanceof GraphQLList) {
} else if (isListType(t)) {
text(into, '[');
// @ts-ignore
renderType(into, t.ofType);
text(into, ']');
} else {
Expand Down
27 changes: 0 additions & 27 deletions resources/buildJs.js

This file was deleted.

52 changes: 0 additions & 52 deletions resources/publishCleanup.ts

This file was deleted.

File renamed without changes.
Loading

0 comments on commit 755d0c4

Please sign in to comment.