From dc012e35676a1baad2b0f196509e01d970e94807 Mon Sep 17 00:00:00 2001 From: jbaxleyiii Date: Thu, 3 Oct 2019 00:13:50 -0400 Subject: [PATCH 1/7] This commit seeks to improve the performance when determining if a field is missing any @client directives when using local state with Apollo Client. Instead of recursing over the entire document for each field, we walk the tree once while checking the schema for any possible client side annotations. This PR is an initial take at improving this workload but doesn't complete the work. Still to be done includes supporting @client validation on fragment spreads --- .../apollo-language-server/__mocks__/fs.ts | 3 + .../NoMissingClientDirectives.test.ts | 225 ++++++++++++++++++ .../src/errors/validation.ts | 200 +++++++++++----- .../src/languageProvider.ts | 9 +- .../src/project/client.ts | 1 - .../src/utilities/graphql.ts | 65 ++++- .../src/utilities/source.ts | 38 --- 7 files changed, 441 insertions(+), 100 deletions(-) create mode 100644 packages/apollo-language-server/__mocks__/fs.ts create mode 100644 packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts diff --git a/packages/apollo-language-server/__mocks__/fs.ts b/packages/apollo-language-server/__mocks__/fs.ts new file mode 100644 index 0000000000..965e2a7046 --- /dev/null +++ b/packages/apollo-language-server/__mocks__/fs.ts @@ -0,0 +1,3 @@ +import { fs } from "memfs"; + +module.exports = fs; diff --git a/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts b/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts new file mode 100644 index 0000000000..b87a58add2 --- /dev/null +++ b/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts @@ -0,0 +1,225 @@ +import { NoMissingClientDirectives } from "../validation"; +import { GraphQLClientProject } from "../../project/client"; +import { readFileSync } from "fs"; +import { basename } from "path"; + +import { vol } from "memfs"; +import { LoadingHandler } from "../../loadingHandler"; +import { ApolloConfig, ClientConfig } from "../../config"; +import URI from "vscode-uri"; + +const serviceSchema = /* GraphQL */ ` + type Query { + me: User + } + + type User { + name: String + friends: [User] + } +`; +const clientSchema = /* GraphQL */ ` + extend type Query { + isOnline: Boolean + } + extend type User { + isLiked: Boolean + localUser: User + } +`; +const a = /* GraphQL */ ` + query a { + isOnline + me { + name + localUser @client { + friends { + isLiked + } + } + friends { + name + isLiked + } + } + } +`; + +const b = /* GraphQL */ ` + query b { + me { + ... { + isLiked + } + ... @client { + localUser { + name + } + } + } + } +`; + +const c = /* GraphQL */ ` + query c { + me { + ...isLiked + } + } + fragment localUser on User @client { + localUser { + name + } + } + fragment isLiked on User { + isLiked + ...localUser + } +`; + +const d = /* GraphQL */ ` + fragment isLiked on User { + isLiked + } + query d { + me { + ...isLiked + ...locaUser + } + } + fragment localUser on User @client { + localUser { + name + } + } +`; + +const e = /* GraphQL */ ` + fragment friends on User { + friends { + ...isLiked + ... on User @client { + localUser { + name + } + } + } + } + query e { + isOnline @client + me { + ...friends + } + } + fragment isLiked on User { + isLiked + } +`; + +// TODO support inline fragment spreads +const f = /* GraphQL */ ` + query f { + me { + ...isLiked @client + } + } + fragment isLiked on User { + isLiked + } +`; + +const rootURI = URI.file(process.cwd()); + +const config = new ApolloConfig({ + client: { + service: { + name: "server", + localSchemaFile: "./schema.graphql" + }, + includes: ["./src/**.graphql"], + excludes: ["./__tests__"], + validationRules: [NoMissingClientDirectives] + }, + engine: {} +}) as ClientConfig; + +class MockLoadingHandler implements LoadingHandler { + handle(_message: string, value: Promise): Promise { + return value; + } + handleSync(_message: string, value: () => T): T { + return value(); + } + showError(_message: string): void {} +} + +jest.mock("fs"); +jest.useFakeTimers(); + +describe("client state", () => { + beforeEach(() => { + vol.fromJSON({ + "apollo.config.js": `module.exports = { + client: { + service: { + localSchemaFile: './schema.graphql' + } + } + }`, + "schema.graphql": serviceSchema, + "src/client-schema.graphql": clientSchema, + "src/a.graphql": a, + "src/b.graphql": b, + "src/c.graphql": c, + "src/d.graphql": d, + "src/e.graphql": e + // "src/f.graphql": f, + }); + }); + afterEach(jest.restoreAllMocks); + + it("should report validation errors for missing @client directives", async () => { + const project = new GraphQLClientProject({ + config, + loadingHandler: new MockLoadingHandler(), + rootURI + }); + + const errors = Object.create(null); + project.onDiagnostics(({ diagnostics, uri }) => { + const path = basename(URI.parse(uri).path); + diagnostics.forEach(({ error }: any) => { + if (!errors[path]) errors[path] = []; + errors[path].push(error); + }); + }); + + await project.whenReady; + + // the project runs validation on construction in an setTimeout(validate, 0) + // to give non blocking diagnostics on langauge server startup + // XXX is this slowing down the CLI? + jest.runOnlyPendingTimers(); + + expect(errors).toMatchInlineSnapshot(` + Object { + "a.graphql": Array [ + [GraphQLError: @client directive is missing on local field "isOnline"], + [GraphQLError: @client directive is missing on local field "isLiked"], + ], + "b.graphql": Array [ + [GraphQLError: @client directive is missing on fragment around local fields "isLiked"], + ], + "c.graphql": Array [ + [GraphQLError: @client directive is missing on fragment "isLiked" around local fields "isLiked,localUser"], + ], + "d.graphql": Array [ + [GraphQLError: @client directive is missing on fragment "isLiked" around local fields "isLiked"], + ], + "e.graphql": Array [ + [GraphQLError: @client directive is missing on fragment "isLiked" around local fields "isLiked"], + ], + } + `); + }); +}); diff --git a/packages/apollo-language-server/src/errors/validation.ts b/packages/apollo-language-server/src/errors/validation.ts index e816d368a4..605aad8a57 100644 --- a/packages/apollo-language-server/src/errors/validation.ts +++ b/packages/apollo-language-server/src/errors/validation.ts @@ -12,17 +12,22 @@ import { visit, visitWithTypeInfo, visitInParallel, - getLocation + getLocation, + InlineFragmentNode, + Kind, + isObjectType } from "graphql"; import { TextEdit } from "vscode-languageserver"; import { ToolError, logError } from "./logger"; import { ValidationRule } from "graphql/validation/ValidationContext"; +import { positionFromSourceLocation } from "../utilities/source"; import { - positionFromSourceLocation, - isFieldResolvedLocally -} from "../utilities/source"; + buildExecutionContext, + ExecutionContext +} from "graphql/execution/execute"; +import { hasClientDirective, simpleCollectFields } from "../utilities/graphql"; export interface CodeActionInfo { message: string; @@ -106,65 +111,148 @@ export function NoTypenameAlias(context: ValidationContext) { }; } +function hasClientSchema(schema: GraphQLSchema): boolean { + const query = schema.getQueryType(); + const mutation = schema.getMutationType(); + const subscription = schema.getSubscriptionType(); + + return Boolean( + (query && query.clientSchema) || + (mutation && mutation.clientSchema) || + (subscription && subscription.clientSchema) + ); +} + export function NoMissingClientDirectives(context: ValidationContext) { const root = context.getDocument(); - return { - Field(node: FieldNode) { - const parentType = context.getParentType(); - const fieldDef = context.getFieldDef(); - - if (!parentType || !fieldDef) return; - - const isClientType = - parentType.clientSchema && - parentType.clientSchema.localFields && - parentType.clientSchema.localFields.includes(fieldDef.name); - - const isResolvedLocally = isFieldResolvedLocally(node, root); - - if (isClientType && !isResolvedLocally) { - let extensions: { [key: string]: any } | null = null; - const nameLoc = node.name.loc; - if (nameLoc) { - let { source, end: locToInsertDirective } = nameLoc; - if (node.arguments && node.arguments.length !== 0) { - // must insert directive after field arguments - const endOfArgs = source.body.indexOf(")", locToInsertDirective); - locToInsertDirective = endOfArgs + 1; - } - const codeAction: CodeActionInfo = { - message: `Add @client directive to "${node.name.value}"`, - edits: [ - TextEdit.insert( - positionFromSourceLocation( - source, - getLocation(source, locToInsertDirective) - ), - " @client" - ) - ] - }; - extensions = { codeAction }; - } + const schema = context.getSchema(); + // early return if we don't have any client fields on the schema + if (!hasClientSchema(schema)) return {}; - context.reportError( - new GraphQLError( - `Local field "${node.name.value}" must have a @client directive`, - [node], - null, - null, - null, - null, - extensions - ) + // this isn't really execution context, but it does group the fragments and operations + // together correctly + // XXX we have a simplified version of this in @apollo/gateway that we could proably use + // intead of this + const executionContext = buildExecutionContext( + schema, + root, + Object.create(null), + Object.create(null), + undefined, + undefined, + undefined + ); + function visitor( + node: FieldNode | InlineFragmentNode | FragmentDefinitionNode + ) { + // In cases where we are looking at a FragmentDefinition, there is no parent type + // but instead, the FragmentDefinition contains the type that we can read from the + // schema + const parentType = + node.kind === Kind.FRAGMENT_DEFINITION + ? schema.getType(node.typeCondition.name.value) + : context.getParentType(); + + const fieldDef = context.getFieldDef(); + + // if we don't have a type to check then we can early return + if (!parentType) return; + + // here we collect all of the fields on a type that are marked "local" + const clientFields = + parentType && + isObjectType(parentType) && + parentType.clientSchema && + parentType.clientSchema.localFields; + + // XXXX in the case of a fragment spread, the directive could be on the fragment definition + let clientDirectivePresent = hasClientDirective(node); + + let message = "@client directive is missing on "; + let selectsClientFieldSet = false; + switch (node.kind) { + case Kind.FIELD: + // fields are simple because we can just see if the name exists in the local fields + // array on the parent type + selectsClientFieldSet = Boolean( + clientFields && clientFields.includes(fieldDef!.name) + ); + message += `local field "${node.name.value}"`; + break; + case Kind.INLINE_FRAGMENT: + case Kind.FRAGMENT_DEFINITION: + // XXX why isn't this type checking below? + if (Array.isArray(executionContext)) break; + + const fields = simpleCollectFields( + executionContext as ExecutionContext, + node.selectionSet, + Object.create(null), + Object.create(null) ); - } - if (isClientType) { - return false; + // once we have a list of fields on the fragment, we can compare them + // to the list of types. The fields within a fragment need to be a + // subset of the overall local fields types + const fieldNames = Object.entries(fields).map(([name]) => name); + selectsClientFieldSet = fieldNames.every( + field => clientFields && clientFields.includes(field) + ); + message += `fragment ${ + "name" in node ? `"${node.name.value}" ` : "" + }around local fields "${fieldNames.join(",")}"`; + break; + } + + // if the field's parent is part of the client schema and that type + // includes a field with the same name as this node, we can see + // if it has an @client directive to resolve locally + if (selectsClientFieldSet && !clientDirectivePresent) { + let extensions: { [key: string]: any } | null = null; + const name = "name" in node && node.name; + // TODO support code actions for inline fragments, fragment spreads, and fragment definitions + if (name && name.loc) { + let { source, end: locToInsertDirective } = name.loc; + if ( + "arguments" in node && + node.arguments && + node.arguments.length !== 0 + ) { + // must insert directive after field arguments + const endOfArgs = source.body.indexOf(")", locToInsertDirective); + locToInsertDirective = endOfArgs + 1; + } + const codeAction: CodeActionInfo = { + message: `Add @client directive to "${name.value}"`, + edits: [ + TextEdit.insert( + positionFromSourceLocation( + source, + getLocation(source, locToInsertDirective) + ), + " @client" + ) + ] + }; + extensions = { codeAction }; } - return; + context.reportError( + new GraphQLError(message, [node], null, null, null, null, extensions) + ); } + + // if we have selected a client field, no need to continue to recurse + if (selectsClientFieldSet) { + return false; + } + + return; + } + return { + InlineFragment: visitor, + FragmentDefinition: visitor, + Field: visitor + // TODO support directives on FragmentSpread }; } diff --git a/packages/apollo-language-server/src/languageProvider.ts b/packages/apollo-language-server/src/languageProvider.ts index f460a967a4..73ffc920a3 100644 --- a/packages/apollo-language-server/src/languageProvider.ts +++ b/packages/apollo-language-server/src/languageProvider.ts @@ -319,10 +319,11 @@ export class GraphQLLanguageProvider { parentType.clientSchema.localFields && parentType.clientSchema.localFields.includes(fieldDef.name); - const isResolvedLocally = isFieldResolvedLocally( - node, - document.ast - ); + const isResolvedLocally = + node.directives && + node.directives.some( + directive => directive.name.value === "client" + ); const content = [ [ diff --git a/packages/apollo-language-server/src/project/client.ts b/packages/apollo-language-server/src/project/client.ts index a913497d65..fdbf56f718 100644 --- a/packages/apollo-language-server/src/project/client.ts +++ b/packages/apollo-language-server/src/project/client.ts @@ -285,7 +285,6 @@ export class GraphQLClientProject extends GraphQLProject { async validate() { if (!this._onDiagnostics) return; - if (!this.serviceSchema) return; const diagnosticSet = new DiagnosticSet(); diff --git a/packages/apollo-language-server/src/utilities/graphql.ts b/packages/apollo-language-server/src/utilities/graphql.ts index 5503294a29..5b9159918a 100644 --- a/packages/apollo-language-server/src/utilities/graphql.ts +++ b/packages/apollo-language-server/src/utilities/graphql.ts @@ -16,9 +16,13 @@ import { DirectiveDefinitionNode, isObjectType, isInterfaceType, - isUnionType + isUnionType, + FragmentDefinitionNode, + InlineFragmentNode } from "graphql"; +import { ExecutionContext } from "graphql/execution/execute"; + export function isNode(maybeNode: any): maybeNode is ASTNode { return maybeNode && typeof maybeNode.kind === "string"; } @@ -345,6 +349,65 @@ export function withTypenameFieldAddedWhereNeeded(ast: ASTNode) { }); } +function getFieldEntryKey(node: FieldNode): string { + return node.alias ? node.alias.value : node.name.value; +} +// this is a simplified verison of the collect fields algorithm that the +// reference implementation uses during execution +// in this case, we don't care about boolean conditions of validating the +// type conditions as other validation has done that already +export function simpleCollectFields( + context: ExecutionContext, + selectionSet: SelectionSetNode, + fields: Record, + visitedFragmentNames: Record +): Record { + for (const selection of selectionSet.selections) { + switch (selection.kind) { + case Kind.FIELD: { + const name = getFieldEntryKey(selection); + if (!fields[name]) { + fields[name] = []; + } + fields[name].push(selection); + break; + } + case Kind.INLINE_FRAGMENT: { + simpleCollectFields( + context, + selection.selectionSet, + fields, + visitedFragmentNames + ); + break; + } + case Kind.FRAGMENT_SPREAD: { + const fragName = selection.name.value; + if (visitedFragmentNames[fragName]) continue; + visitedFragmentNames[fragName] = true; + const fragment = context.fragments[fragName]; + if (!fragment) continue; + simpleCollectFields( + context, + fragment.selectionSet, + fields, + visitedFragmentNames + ); + break; + } + } + } + return fields; +} +export function hasClientDirective( + node: FieldNode | InlineFragmentNode | FragmentDefinitionNode +) { + return ( + node.directives && + node.directives.some(directive => directive.name.value === "client") + ); +} + export interface ClientSchemaInfo { localFields?: string[]; } diff --git a/packages/apollo-language-server/src/utilities/source.ts b/packages/apollo-language-server/src/utilities/source.ts index c161956a3f..81d0ac94e8 100644 --- a/packages/apollo-language-server/src/utilities/source.ts +++ b/packages/apollo-language-server/src/utilities/source.ts @@ -183,41 +183,3 @@ export function getASTNodeAndTypeInfoAtPosition( return null; } } - -export function isFieldResolvedLocally( - fieldNode: FieldNode, - root: ASTNode -): boolean | null { - const { loc } = fieldNode; - if (!loc) return null; - - let underClientDirective: boolean = false; - - function visitor( - node: FieldNode | InlineFragmentNode | FragmentDefinitionNode - ) { - if ( - loc && - node.loc && - node.loc.start <= loc.start && - loc.end <= node.loc.end && - node.directives && - node.directives.some(directive => directive.name.value === "client") - ) { - // If this node's location completely contains the target fieldNode's location, - // and this node has a @client directive, then the fieldNode must be resolved - // locally. We can then stop visiting by returning BREAK. - underClientDirective = true; - return BREAK; - } - return; - } - - visit(root, { - Field: visitor, - InlineFragment: visitor, - FragmentDefinition: visitor - }); - - return underClientDirective; -} From ebe1a714314f972edd2fe4bdef5d55347f1ae25e Mon Sep 17 00:00:00 2001 From: jbaxleyiii Date: Thu, 3 Oct 2019 00:32:33 -0400 Subject: [PATCH 2/7] fix TS error on failure to clean up removed function --- packages/apollo-language-server/src/languageProvider.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/apollo-language-server/src/languageProvider.ts b/packages/apollo-language-server/src/languageProvider.ts index 73ffc920a3..0ada900b0b 100644 --- a/packages/apollo-language-server/src/languageProvider.ts +++ b/packages/apollo-language-server/src/languageProvider.ts @@ -31,8 +31,7 @@ import { positionFromPositionInContainingDocument, rangeForASTNode, getASTNodeAndTypeInfoAtPosition, - positionToOffset, - isFieldResolvedLocally + positionToOffset } from "./utilities/source"; import { From 91e5da448e601e69ab930890f49558ae7eb20f7f Mon Sep 17 00:00:00 2001 From: jbaxleyiii Date: Thu, 3 Oct 2019 14:15:45 -0400 Subject: [PATCH 3/7] Improve startup performance of CLI by removing extra validation Historically, the language server was designed to be used just with the VS Code extension (and other editors) where, after loading the project, the first thing we want to do is validate all of the documents and schema. However, we resuse the base projects (client / server) in the CLI to share common work especially around document management. Due to this, when the CLI starts up, its runs a potentially expensive validation step regardless of if the command has anything to do with validation. Often this is a no-op because of a missing schema, but given the correct env variables that loading can still happen automatically (something to clean up later). This change moves the logic of "auto validate" to the concept of a workspace which is used in long running processes (currently the VS Code but soon the CLI watch mode) and keeps the base project class clean. It also removes the hack in place that helped to highlight this in the recent added test suite for client validation. --- .../src/errors/__tests__/NoMissingClientDirectives.test.ts | 7 +------ packages/apollo-language-server/src/project/base.ts | 1 - packages/apollo-language-server/src/workspace.ts | 5 +++++ packages/apollo/src/commands/client/codegen.ts | 4 ++++ 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts b/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts index b87a58add2..a52d71aa6c 100644 --- a/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts +++ b/packages/apollo-language-server/src/errors/__tests__/NoMissingClientDirectives.test.ts @@ -154,7 +154,6 @@ class MockLoadingHandler implements LoadingHandler { } jest.mock("fs"); -jest.useFakeTimers(); describe("client state", () => { beforeEach(() => { @@ -195,11 +194,7 @@ describe("client state", () => { }); await project.whenReady; - - // the project runs validation on construction in an setTimeout(validate, 0) - // to give non blocking diagnostics on langauge server startup - // XXX is this slowing down the CLI? - jest.runOnlyPendingTimers(); + await project.validate(); expect(errors).toMatchInlineSnapshot(` Object { diff --git a/packages/apollo-language-server/src/project/base.ts b/packages/apollo-language-server/src/project/base.ts index 441e5d28a8..6e2eee8dab 100644 --- a/packages/apollo-language-server/src/project/base.ts +++ b/packages/apollo-language-server/src/project/base.ts @@ -112,7 +112,6 @@ export abstract class GraphQLProject implements GraphQLSchemaProvider { this.readyPromise = Promise.all(this.initialize()) .then(() => { this._isReady = true; - this.invalidate(); }) .catch(error => { console.error(error); diff --git a/packages/apollo-language-server/src/workspace.ts b/packages/apollo-language-server/src/workspace.ts index 7cf5443a7d..ae4070e8f8 100644 --- a/packages/apollo-language-server/src/workspace.ts +++ b/packages/apollo-language-server/src/workspace.ts @@ -90,6 +90,11 @@ export class GraphQLWorkspace { }); } + // after a project has loaded, we do an initial validation to surface errors + // on the start of the language server. Instead of doing this in the + // base class which is used by codegen and other tools + project.whenReady.then(() => project.validate()); + return project; } diff --git a/packages/apollo/src/commands/client/codegen.ts b/packages/apollo/src/commands/client/codegen.ts index cfcd34b44e..4244dde9a4 100644 --- a/packages/apollo/src/commands/client/codegen.ts +++ b/packages/apollo/src/commands/client/codegen.ts @@ -170,6 +170,10 @@ export default class Generate extends ClientCommand { }); if (!schema) throw new Error("Error loading schema"); + // make sure all of the doucuments that we are going to be using for codegen + // are valid documents + project.validate(); + const write = () => { const operations = Object.values(this.project.operations); const fragments = Object.values(this.project.fragments); From 0a6820b960330f6e17c4875a6bf771965b7f6193 Mon Sep 17 00:00:00 2001 From: jbaxleyiii Date: Thu, 3 Oct 2019 14:22:53 -0400 Subject: [PATCH 4/7] update changelog --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 475ab931b4..97cb41c05b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,12 @@ ## Upcoming - `apollo` + - Improve performance of CLI when running projects by delaying or not calling validation [#1559](https://github.com/apollographql/apollo-tooling/pull/1559) - Use "table" package for tabular output and word wrap support [#1524](https://github.com/apollographql/apollo-tooling/pull/1524) - Use new single step mutation for checking federated service schemas [#1539](https://github.com/apollographql/apollo-tooling/pull/1539) - Add support for `localSchemaFile` for federated service commands [#1489](https://github.com/apollographql/apollo-tooling/pull/1489) - `apollo-codegen-core` - - + - Improve performance of validation when client fields are present or not [#1559](https://github.com/apollographql/apollo-tooling/pull/1559) - `apollo-codegen-flow` - - `apollo-codegen-scala` @@ -28,7 +29,7 @@ - `apollo-tools` - - `vscode-apollo` - - + - Improve performance of validation when client fields are present or not [#1559](https://github.com/apollographql/apollo-tooling/pull/1559) ## `apollo@2.18.3` From bc638b2f2d36373a12442e38e97f9ee98d216a69 Mon Sep 17 00:00:00 2001 From: jbaxleyiii Date: Thu, 3 Oct 2019 15:45:21 -0400 Subject: [PATCH 5/7] Pragmatic take on improving codegen efficiency This simplifies the watch and non watch modes by only calling write within the watcher and not relying on the underyling documentChange events from the project base. It also add a *VERY* simple exclude support which is far from the right solution but prevents more loops then currently happen --- .../apollo/src/commands/client/codegen.ts | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/apollo/src/commands/client/codegen.ts b/packages/apollo/src/commands/client/codegen.ts index 4244dde9a4..3d39f7cc88 100644 --- a/packages/apollo/src/commands/client/codegen.ts +++ b/packages/apollo/src/commands/client/codegen.ts @@ -122,9 +122,11 @@ export default class Generate extends ClientCommand { async run() { const { - flags: { watch } + flags: { watch }, + args: { output } } = this.parse(Generate); + let write; const run = () => this.runTasks(({ flags, args, project }) => { let inferredTarget: TargetType = "" as TargetType; @@ -170,11 +172,12 @@ export default class Generate extends ClientCommand { }); if (!schema) throw new Error("Error loading schema"); - // make sure all of the doucuments that we are going to be using for codegen - // are valid documents - project.validate(); - const write = () => { + write = () => { + // make sure all of the doucuments that we are going to be using for codegen + // are valid documents + project.validate(); + const operations = Object.values(this.project.operations); const fragments = Object.values(this.project.fragments); @@ -220,11 +223,6 @@ export default class Generate extends ClientCommand { ); }; - // project.validationDidFinish(write); - project.onDiagnostics(({ uri }) => { - write(); - }); - const writtenFiles = write(); task.title = `Generating query files with '${inferredTarget}' target - wrote ${writtenFiles} files`; @@ -236,13 +234,19 @@ export default class Generate extends ClientCommand { if (watch) { await run().catch(() => {}); const watcher = new Gaze(this.project.config.client.includes); + // FIXME: support excludes with the glob watcher.on("all", (event, file) => { + // don't trigger write events for generated file changes + if (file.indexOf("__generated__") > -1) return; + // don't trigger write events on single output file + if (file.indexOf(output) > -1) return; console.log("\nChange detected, generating types..."); - this.project.fileDidChange(URI.file(file).toString()); + write(); }); if (tty.isatty((process.stdin as any).fd)) { await waitForKey(); watcher.close(); + process.exit(0); } return; } else { From 11f869a09bdad7dd7ecc9a04ea8848c4b3fa4e9d Mon Sep 17 00:00:00 2001 From: Jake Dawkins Date: Fri, 4 Oct 2019 09:05:21 -0400 Subject: [PATCH 6/7] Add gitContext to checkPartialSchema mutation (#1560) * add gitContext to federated checkPartialSchema mutation --- packages/apollo-codegen-core/package.json | 2 +- packages/apollo-codegen-flow/package.json | 2 +- packages/apollo-codegen-scala/package.json | 2 +- packages/apollo-codegen-swift/package.json | 2 +- packages/apollo-codegen-typescript/package.json | 2 +- packages/apollo-graphql/package.json | 2 +- packages/apollo-language-server/package.json | 2 +- packages/apollo/README.md | 2 +- packages/apollo/package.json | 2 +- packages/apollo/src/commands/service/check.ts | 5 ++--- packages/vscode-apollo/package.json | 2 +- 11 files changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/apollo-codegen-core/package.json b/packages/apollo-codegen-core/package.json index ca2aaf98a1..1272a61751 100644 --- a/packages/apollo-codegen-core/package.json +++ b/packages/apollo-codegen-core/package.json @@ -1,7 +1,7 @@ { "name": "apollo-codegen-core", "description": "Core generator APIs for Apollo Codegen", - "version": "0.36.0-alpha.1", + "version": "0.36.0-alpha.2", "author": "Apollo GraphQL ", "license": "MIT", "repository": { diff --git a/packages/apollo-codegen-flow/package.json b/packages/apollo-codegen-flow/package.json index 869cba3442..c3e8546a7e 100644 --- a/packages/apollo-codegen-flow/package.json +++ b/packages/apollo-codegen-flow/package.json @@ -1,7 +1,7 @@ { "name": "apollo-codegen-flow", "description": "Flow generator module for Apollo Codegen", - "version": "0.34.0-alpha.1", + "version": "0.34.0-alpha.2", "author": "Apollo GraphQL ", "license": "MIT", "repository": { diff --git a/packages/apollo-codegen-scala/package.json b/packages/apollo-codegen-scala/package.json index 8c4130f2f0..65cf4e8f2d 100644 --- a/packages/apollo-codegen-scala/package.json +++ b/packages/apollo-codegen-scala/package.json @@ -1,7 +1,7 @@ { "name": "apollo-codegen-scala", "description": "Scala generator module for Apollo Codegen", - "version": "0.35.0-alpha.1", + "version": "0.35.0-alpha.2", "author": "Apollo GraphQL ", "license": "MIT", "repository": { diff --git a/packages/apollo-codegen-swift/package.json b/packages/apollo-codegen-swift/package.json index fcbb71bcb5..19adcf32e9 100644 --- a/packages/apollo-codegen-swift/package.json +++ b/packages/apollo-codegen-swift/package.json @@ -1,7 +1,7 @@ { "name": "apollo-codegen-swift", "description": "Swift generator module for Apollo Codegen", - "version": "0.36.0-alpha.1", + "version": "0.36.0-alpha.2", "author": "Apollo GraphQL ", "license": "MIT", "repository": { diff --git a/packages/apollo-codegen-typescript/package.json b/packages/apollo-codegen-typescript/package.json index 4993715e41..656906ed39 100644 --- a/packages/apollo-codegen-typescript/package.json +++ b/packages/apollo-codegen-typescript/package.json @@ -1,7 +1,7 @@ { "name": "apollo-codegen-typescript", "description": "TypeScript generator module for Apollo Codegen", - "version": "0.36.0-alpha.1", + "version": "0.36.0-alpha.2", "author": "Apollo GraphQL ", "license": "MIT", "repository": { diff --git a/packages/apollo-graphql/package.json b/packages/apollo-graphql/package.json index ca3d864bcb..f8315ff384 100644 --- a/packages/apollo-graphql/package.json +++ b/packages/apollo-graphql/package.json @@ -1,6 +1,6 @@ { "name": "apollo-graphql", - "version": "0.3.4-alpha.0", + "version": "0.3.4-alpha.1", "description": "Apollo GraphQL utility library", "main": "lib/index.js", "types": "lib/index.d.ts", diff --git a/packages/apollo-language-server/package.json b/packages/apollo-language-server/package.json index a1d05d0704..0ae48838e6 100644 --- a/packages/apollo-language-server/package.json +++ b/packages/apollo-language-server/package.json @@ -1,7 +1,7 @@ { "name": "apollo-language-server", "description": "A language server for Apollo GraphQL projects", - "version": "1.16.0-alpha.1", + "version": "1.16.0-alpha.2", "author": "Apollo GraphQL ", "license": "MIT", "repository": { diff --git a/packages/apollo/README.md b/packages/apollo/README.md index d3b723c33a..730bde9f4d 100644 --- a/packages/apollo/README.md +++ b/packages/apollo/README.md @@ -21,7 +21,7 @@ $ npm install -g apollo $ apollo COMMAND running command... $ apollo (-v|--version|version) -apollo/2.19.0-alpha.1 darwin-x64 node-v8.11.1 +apollo/2.19.0-alpha.2 darwin-x64 node-v8.11.1 $ apollo --help [COMMAND] USAGE $ apollo COMMAND diff --git a/packages/apollo/package.json b/packages/apollo/package.json index 8201816497..e66d0ed0d9 100644 --- a/packages/apollo/package.json +++ b/packages/apollo/package.json @@ -1,7 +1,7 @@ { "name": "apollo", "description": "Command line tool for Apollo GraphQL", - "version": "2.19.0-alpha.1", + "version": "2.19.0-alpha.2", "referenceID": "21ad0845-c235-422e-be7d-a998310df972", "author": "Apollo GraphQL ", "license": "MIT", diff --git a/packages/apollo/src/commands/service/check.ts b/packages/apollo/src/commands/service/check.ts index 12b44c9d9d..75a671784a 100644 --- a/packages/apollo/src/commands/service/check.ts +++ b/packages/apollo/src/commands/service/check.ts @@ -357,7 +357,8 @@ export default class ServiceCheck extends ProjectCommand { implementingServiceName: serviceName, partialSchema: { sdl - } + }, + gitContext: await gitInfo(this.log) }); task.title = `Found ${pluralize( @@ -437,8 +438,6 @@ export default class ServiceCheck extends ProjectCommand { .__schema as IntrospectionSchemaInput }; - await gitInfo(this.log); - const historicParameters = validateHistoricParams({ validationPeriod: flags.validationPeriod, queryCountThreshold: flags.queryCountThreshold, diff --git a/packages/vscode-apollo/package.json b/packages/vscode-apollo/package.json index e44ee2062d..b02c5a506b 100644 --- a/packages/vscode-apollo/package.json +++ b/packages/vscode-apollo/package.json @@ -2,7 +2,7 @@ "name": "vscode-apollo", "displayName": "Apollo GraphQL", "description": "Rich editor support for GraphQL client and server development that seamlessly integrates with the Apollo platform", - "version": "1.11.0-alpha.1", + "version": "1.11.0-alpha.2", "referenceID": "87197759-7617-40d0-b32e-46d378e907c7", "author": "Apollo GraphQL ", "license": "MIT", From 29d8f12ec8db6c59f3d0fb5736312eba95fcb28b Mon Sep 17 00:00:00 2001 From: Jake Date: Fri, 4 Oct 2019 09:08:22 -0400 Subject: [PATCH 7/7] Clarify changelog and fix typo --- CHANGELOG.md | 2 +- packages/apollo-language-server/src/errors/validation.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97cb41c05b..e229eb76cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Upcoming - `apollo` - - Improve performance of CLI when running projects by delaying or not calling validation [#1559](https://github.com/apollographql/apollo-tooling/pull/1559) + - Improve performance of CLI when running projects by delaying or not calling validation unnecessarily [#1559](https://github.com/apollographql/apollo-tooling/pull/1559) - Use "table" package for tabular output and word wrap support [#1524](https://github.com/apollographql/apollo-tooling/pull/1524) - Use new single step mutation for checking federated service schemas [#1539](https://github.com/apollographql/apollo-tooling/pull/1539) - Add support for `localSchemaFile` for federated service commands [#1489](https://github.com/apollographql/apollo-tooling/pull/1489) diff --git a/packages/apollo-language-server/src/errors/validation.ts b/packages/apollo-language-server/src/errors/validation.ts index 605aad8a57..95c4674996 100644 --- a/packages/apollo-language-server/src/errors/validation.ts +++ b/packages/apollo-language-server/src/errors/validation.ts @@ -131,7 +131,7 @@ export function NoMissingClientDirectives(context: ValidationContext) { // this isn't really execution context, but it does group the fragments and operations // together correctly - // XXX we have a simplified version of this in @apollo/gateway that we could proably use + // XXX we have a simplified version of this in @apollo/gateway that we could probably use // intead of this const executionContext = buildExecutionContext( schema,