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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debounce schema change events to fix codegen bugs #3647

Merged
merged 6 commits into from
Jul 9, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/eighty-maps-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'graphql-language-service-server': patch
'graphql-language-service-cli': patch
'vscode-graphql': patch
---

**Bugfixes**

debounce schema change events to fix codegen bugs to fix #3622

on mass file changes, network schema is overfetching because the schema cache is now invalidated on every watched schema file change

to address this, we debounce the new `onSchemaChange` event by 400ms

note that `schemaCacheTTL` can only be set in extension settings or graphql config at the top level - it will be ignored if configured per-project in the graphql config

**Code Improvements**

- Fixes flaky tests, and `schemaCacheTTL` setting not being passed to the cache
- Adds a test to validate network schema changes are reflected in the cache
19 changes: 11 additions & 8 deletions packages/graphql-language-service-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ module.exports = {
// a function that returns an array of validation rules, ala https://github.com/graphql/graphql-js/tree/main/src/validation/rules
// note that this file will be loaded by the vscode runtime, so the node version and other factors will come into play
customValidationRules: require('./config/customValidationRules'),
schemaCacheTTL: 1000, // reduce or increase minimum schema cache lifetime from 30000ms (30 seconds). you may want to reduce this if you are developing fullstack with network schema
languageService: {
// this is enabled by default if non-local files are specified in the project `schema`
// NOTE: this will disable all definition lookup for local SDL files
Expand Down Expand Up @@ -257,14 +258,16 @@ via `initializationOptions` in nvim.coc. The options are mostly designed to
configure graphql-config's load parameters, the only thing we can't configure
with graphql config. The final option can be set in `graphql-config` as well

| Parameter | Default | Description |
| ----------------------------------------- | ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `graphql-config.load.baseDir` | workspace root or process.cwd() | the path where graphql config looks for config files |
| `graphql-config.load.filePath` | `null` | exact filepath of the config file. |
| `graphql-config.load.configName` | `graphql` | config name prefix instead of `graphql` |
| `graphql-config.load.legacy` | `true` | backwards compatibility with `graphql-config@2` |
| `graphql-config.dotEnvPath` | `null` | backwards compatibility with `graphql-config@2` |
| `vscode-graphql.cacheSchemaFileForLookup` | `true` if `schema` contains non-sdl files or urls | generate an SDL file based on your graphql-config schema configuration for schema definition lookup and other features. enabled by default when your `schema` config are urls or introspection json, or if you have any non-local SDL files in `schema` |
| Parameter | Default | Description |
| ----------------------------------------- | ------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `graphql-config.load.baseDir` | workspace root or process.cwd() | the path where graphql config looks for config files |
| `graphql-config.load.filePath` | `null` | exact filepath of the config file. |
| `graphql-config.load.configName` | `graphql` | config name prefix instead of `graphql` |
| `graphql-config.load.legacy` | `true` | backwards compatibility with `graphql-config@2` |
| `graphql-config.dotEnvPath` | `null` | backwards compatibility with `graphql-config@2` |
| `vscode-graphql.cacheSchemaFileForLookup` | `true` if `schema` contains non-SDL files or URLs | generate an SDL file based on your graphql-config schema configuration for definition lookup and other features. enabled by default when your `schema` config are URLs or introspection JSON, or if you have any non-local SDL files in `schema` |
| `vscode-graphql.schemaCacheTTL` | `30000` | an integer value in milliseconds for the desired minimum cache lifetime for all schemas, which also causes the generated file to be re-written. set to 30s by default. effectively a "lazy" form of polling. if you are developing a schema alongside client queries, you may want to decrease this |
| `vscode-graphql.debug` | `false` | show more verbose log output in the output channel |

all the `graphql-config.load.*` configuration values come from static
`loadConfig()` options in graphql config.
Expand Down
1 change: 1 addition & 0 deletions packages/graphql-language-service-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"@types/mkdirp": "^1.0.1",
"@types/mock-fs": "^4.13.4",
"cross-env": "^7.0.2",
"debounce-promise": "^3.1.2",
"graphql": "^16.8.1",
"mock-fs": "^5.2.0"
}
Expand Down
45 changes: 20 additions & 25 deletions packages/graphql-language-service-server/src/GraphQLCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
} from './constants';
import { NoopLogger, Logger } from './Logger';
import { LRUCache } from 'lru-cache';
// import { is } from '@babel/types';

const codeLoaderConfig: CodeFileLoaderConfig = {
noSilentErrors: false,
Expand Down Expand Up @@ -83,12 +84,14 @@ export async function getGraphQLCache({
loadConfigOptions,
config,
onSchemaChange,
schemaCacheTTL,
}: {
parser: typeof parseDocument;
logger: Logger | NoopLogger;
loadConfigOptions: LoadConfigOptions;
config?: GraphQLConfig;
onSchemaChange?: OnSchemaChange;
schemaCacheTTL?: number;
}): Promise<GraphQLCache> {
const graphQLConfig =
config ||
Expand All @@ -105,6 +108,10 @@ export async function getGraphQLCache({
parser,
logger,
onSchemaChange,
schemaCacheTTL:
schemaCacheTTL ??
// @ts-expect-error TODO: add types for extension configs
config?.extensions?.get('languageService')?.schemaCacheTTL,
});
}

Expand All @@ -119,26 +126,29 @@ export class GraphQLCache {
_parser: typeof parseDocument;
_logger: Logger | NoopLogger;
_onSchemaChange?: OnSchemaChange;
_schemaCacheTTL?: number;

constructor({
configDir,
config,
parser,
logger,
onSchemaChange,
schemaCacheTTL,
}: {
configDir: Uri;
config: GraphQLConfig;
parser: typeof parseDocument;
logger: Logger | NoopLogger;
onSchemaChange?: OnSchemaChange;
schemaCacheTTL?: number;
}) {
this._configDir = configDir;
this._graphQLConfig = config;
this._graphQLFileListCache = new Map();
this._schemaMap = new LRUCache({
max: 20,
ttl: 1000 * 30,
ttl: schemaCacheTTL ?? 1000 * 30,
ttlAutopurge: true,
updateAgeOnGet: false,
});
Expand Down Expand Up @@ -602,10 +612,10 @@ export class GraphQLCache {
}

getSchema = async (
projectName: string,
appName?: string,
queryHasExtensions?: boolean | null,
): Promise<GraphQLSchema | null> => {
const projectConfig = this._graphQLConfig.getProject(projectName);
const projectConfig = this._graphQLConfig.getProject(appName);

if (!projectConfig) {
return null;
Expand All @@ -620,35 +630,18 @@ export class GraphQLCache {
if (schemaPath && schemaKey) {
schemaCacheKey = schemaKey as string;

try {
// Read from disk
schema = await projectConfig.loadSchema(
projectConfig.schema,
'GraphQLSchema',
{
assumeValid: true,
assumeValidSDL: true,
experimentalFragmentVariables: true,
sort: false,
includeSources: true,
allowLegacySDLEmptyFields: true,
allowLegacySDLImplementsInterfaces: true,
},
);
} catch {
// // if there is an error reading the schema, just use the last valid schema
schema = this._schemaMap.get(schemaCacheKey);
}

// Maybe use cache
if (this._schemaMap.has(schemaCacheKey)) {
schema = this._schemaMap.get(schemaCacheKey);

if (schema) {
return queryHasExtensions
? this._extendSchema(schema, schemaPath, schemaCacheKey)
: schema;
}
}

// Read from disk
schema = await projectConfig.getSchema();
}

const customDirectives = projectConfig?.extensions?.customDirectives;
Expand All @@ -667,7 +660,9 @@ export class GraphQLCache {

if (schemaCacheKey) {
this._schemaMap.set(schemaCacheKey, schema);
await this._onSchemaChange?.(projectConfig);
if (this._onSchemaChange) {
this._onSchemaChange(projectConfig);
}
}
return schema;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import { NoopLogger, Logger } from './Logger';
import glob from 'fast-glob';
import { isProjectSDLOnly, unwrapProjectSchema } from './common';
import { DefinitionQueryResponse } from 'graphql-language-service/src/interface';
import { default as debounce } from 'debounce-promise';

const configDocLink =
'https://www.npmjs.com/package/graphql-language-service-server#user-content-graphql-configuration-file';
Expand Down Expand Up @@ -229,7 +230,7 @@ export class MessageProcessor {
rootDir,
};

const onSchemaChange = async (project: GraphQLProjectConfig) => {
const onSchemaChange = debounce(async (project: GraphQLProjectConfig) => {
const { cacheSchemaFileForLookup } =
this.getCachedSchemaSettings(project);
if (!cacheSchemaFileForLookup) {
Expand All @@ -241,7 +242,7 @@ export class MessageProcessor {
return;
}
return this.cacheConfigSchemaFile(project);
};
}, 400);

try {
// now we have the settings so we can re-build the logger
Expand All @@ -255,6 +256,7 @@ export class MessageProcessor {
parser: this._parser,
configDir: rootDir,
onSchemaChange,
schemaCacheTTL: this._settings?.schemaCacheTTL,
});
this._languageService = new GraphQLLanguageService(
this._graphQLCache,
Expand All @@ -267,6 +269,7 @@ export class MessageProcessor {
loadConfigOptions: this._loadConfigOptions,
logger: this._logger,
onSchemaChange,
schemaCacheTTL: this._settings?.schemaCacheTTL,
});
this._languageService = new GraphQLLanguageService(
this._graphQLCache,
Expand Down Expand Up @@ -1307,6 +1310,7 @@ export class MessageProcessor {
unwrapProjectSchema(project).map(async schema => {
const schemaFilePath = path.resolve(project.dirpath, schema);
const uriFilePath = URI.parse(uri).fsPath;

if (uriFilePath === schemaFilePath) {
try {
const file = await readFile(schemaFilePath, 'utf-8');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,16 @@ import { serializeRange } from './__utils__/utils';
import { readFile } from 'node:fs/promises';
import { existsSync } from 'node:fs';
import { URI } from 'vscode-uri';
import { GraphQLSchema, introspectionFromSchema } from 'graphql';
import {
GraphQLSchema,
buildASTSchema,
introspectionFromSchema,
parse,
} from 'graphql';
import fetchMock from 'fetch-mock';

const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));

jest.mock('@whatwg-node/fetch', () => {
const { AbortController } = require('node-abort-controller');

Expand All @@ -26,7 +33,7 @@ const mockSchema = (schema: GraphQLSchema) => {
descriptions: true,
}),
};
fetchMock.mock({
return fetchMock.mock({
matcher: '*',
response: {
headers: {
Expand Down Expand Up @@ -523,17 +530,37 @@ describe('MessageProcessor with config', () => {
],
schemaFile,
],
settings: { schemaCacheTTL: 500 },
});

const initParams = await project.init('a/query.ts');
expect(initParams.diagnostics[0].message).toEqual('Unknown fragment "T".');

expect(project.lsp._logger.error).not.toHaveBeenCalled();
expect(await project.lsp._graphQLCache.getSchema('a')).toBeDefined();

fetchMock.restore();
mockSchema(
buildASTSchema(
parse(
'type example100 { string: String } type Query { example: example100 }',
),
),
);
await project.lsp.handleWatchedFilesChangedNotification({
changes: [
{ uri: project.uri('a/fragments.ts'), type: FileChangeType.Changed },
],
});
await sleep(1000);
expect(
(await project.lsp._graphQLCache.getSchema('a')).getType('example100'),
).toBeTruthy();
await sleep(1000);
const file = await readFile(join(genSchemaPath.replace('default', 'a')), {
encoding: 'utf-8',
});
expect(file.split('\n').length).toBeGreaterThan(10);
expect(file).toContain('example100');
// add a new typescript file with empty query to the b project
// and expect autocomplete to only show options for project b
await project.addFile(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class MockProject {
}: {
files: Files;
root?: string;
settings?: [name: string, vale: any][];
settings?: Record<string, any>;
}) {
this.root = root;
this.fileCache = new Map(files);
Expand Down Expand Up @@ -100,7 +100,11 @@ export class MockProject {
});
}
private mockFiles() {
const mockFiles = { ...defaultMocks };
const mockFiles = {
...defaultMocks,
// without this, the generated schema file may not be cleaned up by previous tests
'/tmp/graphql-language-service': mockfs.directory(),
};
for (const [filename, text] of this.fileCache) {
mockFiles[this.filePath(filename)] = text;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/graphql-language-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@
"graphql": "^15.5.0 || ^16.0.0"
},
"dependencies": {
"debounce-promise": "^3.1.2",
"nullthrows": "^1.0.0",
"vscode-languageserver-types": "^3.17.1"
},
"devDependencies": {
"@types/benchmark": "^1.0.33",
"@types/debounce-promise": "^3.1.9",
"@types/json-schema": "7.0.9",
"@types/picomatch": "^2.3.0",
"benchmark": "^2.1.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/vscode-graphql-syntax/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
"vscode-oniguruma": "^1.7.0",
"vscode-textmate": "^9.0.0",
"ovsx": "^0.3.0",
"@vscode/vsce": "^2.23.0"
"@vscode/vsce": "^2.22.1-2"
},
"homepage": "https://github.com/graphql/graphiql/blob/main/packages/vscode-graphql-syntax/README.md",
"scripts": {
Expand Down
Loading
Loading