diff --git a/.changeset/twelve-peas-study.md b/.changeset/twelve-peas-study.md new file mode 100644 index 00000000000..d96abbf7fc0 --- /dev/null +++ b/.changeset/twelve-peas-study.md @@ -0,0 +1,6 @@ +--- +'graphql-language-service-server': patch +'vscode-graphql': patch +--- + +fix graphql config init bug diff --git a/packages/graphql-language-service-server/src/MessageProcessor.ts b/packages/graphql-language-service-server/src/MessageProcessor.ts index 6d014e98d93..25a5bba8b68 100644 --- a/packages/graphql-language-service-server/src/MessageProcessor.ts +++ b/packages/graphql-language-service-server/src/MessageProcessor.ts @@ -216,19 +216,12 @@ export class MessageProcessor { } async handleDidChangeConfiguration( - params: DidChangeConfigurationParams, + _params: DidChangeConfigurationParams, ): Promise { - if (!params?.settings || params?.settings.length === 0) { - return {}; - } - - // reset all the workspace caches - // and prepare for them to lazily re-build based - // on where the user opens and saves files await Promise.all( - Array.from(this._processors.values()).map(async processor => { - await processor._initializeConfig(); - }), + Array.from(this._processors.values()).map(processor => + processor._updateGraphQLConfig(), + ), ); this._logger.log( JSON.stringify({ diff --git a/packages/graphql-language-service-server/src/WorkspaceMessageProcessor.ts b/packages/graphql-language-service-server/src/WorkspaceMessageProcessor.ts index 168fbac3bb9..e0df95ac524 100644 --- a/packages/graphql-language-service-server/src/WorkspaceMessageProcessor.ts +++ b/packages/graphql-language-service-server/src/WorkspaceMessageProcessor.ts @@ -8,9 +8,7 @@ */ import glob from 'fast-glob'; -import { existsSync } from 'fs'; -import { readFile, writeFile } from 'fs/promises'; - +import { existsSync, readFileSync, writeFile, writeFileSync } from 'fs'; import { CachedContent, FileChangeTypeKind, @@ -68,9 +66,12 @@ import { LoaderNoResultError, ProjectNotFoundError, } from 'graphql-config'; +import { promisify } from 'util'; import { Logger } from './Logger'; import type { LoadConfigOptions } from './types'; +const writeFileAsync = promisify(writeFile); + const configDocLink = 'https://www.npmjs.com/package/graphql-language-service-server#user-content-graphql-configuration-file'; @@ -81,19 +82,6 @@ type CachedDocumentType = { function toPosition(position: VscodePosition): IPosition { return new Position(position.line, position.character); } -/** - * TODO: much of the logic in here that was added after 2020 or later - * later on should be moved to GraphQLLanguageService or GraphQLCache - * Also - test coverage took a hit with the init "cache warm" - * - * (see `MessageProcessor` file history where this came from) - */ - -enum ProjectConfigCacheStatus { - 'Loading' = 1, - 'Loaded' = 2, - 'Error' = 3, -} export class WorkspaceMessageProcessor { _connection: Connection; @@ -102,7 +90,6 @@ export class WorkspaceMessageProcessor { _languageService!: GraphQLLanguageService; _textDocumentCache: Map; _isInitialized: boolean; - _projectCacheStatuses: Map; _isGraphQLConfigMissing: boolean | null = null; _logger: Logger; _extensions?: GraphQLExtensionDeclaration[]; @@ -140,10 +127,6 @@ export class WorkspaceMessageProcessor { this._tmpDirBase = path.join(this._tmpDir, 'graphql-language-service'); // use legacy mode by default for backwards compatibility this._loadConfigOptions = { legacy: true, ...loadConfigOptions }; - // track whether we've warmed the cache for each project - // much more performant than loading them all at once when - // you load the first file s - this._projectCacheStatuses = new Map(); if ( loadConfigOptions.extensions && loadConfigOptions.extensions?.length > 0 @@ -156,7 +139,8 @@ export class WorkspaceMessageProcessor { } this._rootPath = rootPath; } - async _initializeConfig() { + + async _updateGraphQLConfig() { const settings = await this._connection.workspace.getConfiguration({ section: 'graphql-config', scopeUri: this._rootPath, @@ -193,84 +177,20 @@ export class WorkspaceMessageProcessor { this._graphQLCache, this._logger, ); - this._projectCacheStatuses = new Map(); - - // reset the project config caches so that they rebuild lazily - } catch (err) { - this._handleConfigError({ err }); - } - } - /** - * - * @param uri - */ - async _loadAndCacheProjectConfig(project: GraphQLProjectConfig) { - if (!this._graphQLCache) { - await this._initializeConfig(); - } - if (this._graphQLCache) { - // TODO: we can do some really neat things with these statuses - // if we can send them to the client - try { - this._setProjectCacheStatus( - project.name, - ProjectConfigCacheStatus.Loading, - ); - - await this._cacheSchemaFilesForProject(project); - await this._cacheDocumentFilesforProject(project); - - this._setProjectCacheStatus( - project.name, - ProjectConfigCacheStatus.Loaded, - ); - } catch (err) { - this._setProjectCacheStatus( - project.name, - ProjectConfigCacheStatus.Error, - ); - } - } - } - - // encapsulate this for fun later 😜 - _setProjectCacheStatus( - projectName: string, - cacheStatus: ProjectConfigCacheStatus, - ) { - this._logger.info( - `re-initializing project cache for ${projectName}: ${cacheStatus}`, - ); - // TODO: tell the client about this for visual feedback some day? - // this._connection.sendNotification() - this._projectCacheStatuses.set(projectName, cacheStatus); - } - async _ensureProjectCached(project: GraphQLProjectConfig) { - // Lazily warm the cache when a file is opened or saved in that project - // this - try { - // only check has() here rather than the status, - // otherwise bad configs might try to keep re-loading, - // or pending requests might be duplicated or worse - // this means we only try once per project config - if ( - this._isGraphQLConfigMissing !== true && - !this._projectCacheStatuses?.has(project.name) - ) { - await this._loadAndCacheProjectConfig(project); - } else { - // don't try to initialize again if we've already tried - // and the graphql config file or package.json entry isn't even there - // then initial call to load the config for this file - return null; + if (this._graphQLConfig || this._graphQLCache?.getGraphQLConfig) { + const config = + this._graphQLConfig ?? this._graphQLCache.getGraphQLConfig(); + await this._cacheAllProjectFiles(config); } + this._isInitialized = true; } catch (err) { - this._logger.error(String(err)); + this._handleConfigError({ err }); } } - _handleConfigError({ err }: { err: unknown; uri?: string }) { if (err instanceof ConfigNotFoundError) { + // TODO: obviously this needs to become a map by workspace from uri + // for workspaces support this._isGraphQLConfigMissing = true; this._logConfigError(err.message); @@ -320,12 +240,20 @@ export class WorkspaceMessageProcessor { * Initialize the LSP server when the first file is opened or saved, * so that we can access the user settings for config rootDir, etc */ - - const project = this._graphQLCache.getProjectForFile( - params.textDocument.uri, - ); - - await this._ensureProjectCached(project); + try { + if (!this._isInitialized || !this._graphQLCache) { + // don't try to initialize again if we've already tried + // and the graphql config file or package.json entry isn't even there + if (this._isGraphQLConfigMissing !== true) { + // then initial call to update graphql config + await this._updateGraphQLConfig(); + } else { + return null; + } + } + } catch (err) { + this._logger.error(String(err)); + } // Here, we set the workspace settings in memory, // and re-initialize the language service when a different @@ -352,25 +280,15 @@ export class WorkspaceMessageProcessor { await this._invalidateCache(textDocument, uri, contents); } else { - let hasPkgConfig = false; - try { - hasPkgConfig = - uri.endsWith('package.json') ?? - JSON.parse(await readFile(uri, 'utf-8'))?.graphql; - } catch { - // if package.json is saved with a parsing error, let's ignore it - } const configMatchers = [ 'graphql.config', 'graphqlrc', + 'package.json', this._settings.load?.fileName, ].filter(Boolean); - if (configMatchers.some(v => uri.match(v)?.length) || hasPkgConfig) { - this._logger.info( - "graphql config changed. opening or saving a file will now re-initialize it's project cache", - ); - this._initializeConfig(); - + if (configMatchers.some(v => uri.match(v)?.length)) { + this._logger.info('updating graphql config'); + this._updateGraphQLConfig(); return { uri, diagnostics: [] }; } // update graphql config only when graphql config is saved! @@ -379,12 +297,13 @@ export class WorkspaceMessageProcessor { contents = cachedDocument.contents; } } - if (this._isGraphQLConfigMissing) { + if (!this._graphQLCache) { return { uri, diagnostics }; } else { try { + const project = this._graphQLCache.getProjectForFile(uri); if ( - this._projectCacheStatuses.get(project.name) && + this._isInitialized && project?.extensions?.languageService?.enableValidation !== false ) { await Promise.all( @@ -422,7 +341,7 @@ export class WorkspaceMessageProcessor { async handleDidChangeNotification( params: DidChangeTextDocumentParams, ): Promise { - if (this._isGraphQLConfigMissing ?? !this._graphQLCache) { + if (!this._isInitialized || !this._graphQLCache) { return null; } // For every `textDocument/didChange` event, keep a cache of textDocuments @@ -439,19 +358,14 @@ export class WorkspaceMessageProcessor { '`textDocument`, `textDocument.uri`, and `contentChanges` arguments are required.', ); } - const uri = params.textDocument.uri; + const textDocument = params.textDocument; const contentChanges = params.contentChanges; const contentChange = contentChanges[contentChanges.length - 1]; - const project = this._graphQLCache.getProjectForFile(uri); - await this._ensureProjectCached(project); - - // TODO: is this a good idea? what if a bulk change is incomplete? - // we should re-parse the entire document string - // As `contentChanges` is an array and we just want the // latest update to the text, grab the last entry from the array. + const uri = textDocument.uri; // If it's a .js file, try parsing the contents to see if GraphQL queries // exist. If not found, delete from the cache. @@ -468,6 +382,7 @@ export class WorkspaceMessageProcessor { await this._updateFragmentDefinition(uri, contents); await this._updateObjectTypeDefinition(uri, contents); + const project = this._graphQLCache.getProjectForFile(uri); const diagnostics: Diagnostic[] = []; if (project?.extensions?.languageService?.enableValidation !== false) { @@ -501,7 +416,7 @@ export class WorkspaceMessageProcessor { } handleDidCloseNotification(params: DidCloseTextDocumentParams): void { - if (this._isGraphQLConfigMissing ?? !this._graphQLCache) { + if (!this._isInitialized || !this._graphQLCache) { return; } // For every `textDocument/didClose` event, delete the cached entry. @@ -654,19 +569,16 @@ export class WorkspaceMessageProcessor { change.type === FileChangeTypeKind.Changed ) { const uri = change.uri; - const project = this._graphQLCache.getProjectForFile(uri); - await this._ensureProjectCached(project); - const text = await readFile(URI.parse(uri).fsPath, 'utf-8'); + const text = readFileSync(URI.parse(uri).fsPath, 'utf-8'); const contents = this._parser(text, uri); - // first update cache await this._updateFragmentDefinition(uri, contents); await this._updateObjectTypeDefinition(uri, contents); + const project = this._graphQLCache.getProjectForFile(uri); let diagnostics: Diagnostic[] = []; - // then validate if (project?.extensions?.languageService?.enableValidation !== false) { diagnostics = ( await Promise.all( @@ -894,7 +806,7 @@ export class WorkspaceMessageProcessor { if (schemaDocument) { version = schemaDocument.version++; } - const schemaText = await readFile(uri, { encoding: 'utf-8' }); + const schemaText = readFileSync(uri, { encoding: 'utf-8' }); this._cacheSchemaText(schemaUri, schemaText, version); } } @@ -1026,14 +938,14 @@ export class WorkspaceMessageProcessor { const cachedSchemaDoc = this._getCachedDocument(uri); if (!cachedSchemaDoc) { - await writeFile(fsPath, schemaText, { + await writeFileAsync(fsPath, schemaText, { encoding: 'utf-8', }); await this._cacheSchemaText(uri, schemaText, 1); } // do we have a change in the getSchema result? if so, update schema cache if (cachedSchemaDoc) { - await writeFile(fsPath, schemaText, { + writeFileSync(fsPath, schemaText, { encoding: 'utf-8', }); await this._cacheSchemaText( diff --git a/packages/graphql-language-service-server/src/__tests__/GraphQLCache-test.ts b/packages/graphql-language-service-server/src/__tests__/GraphQLCache-test.ts index d3ca6a95999..debe9ea6162 100644 --- a/packages/graphql-language-service-server/src/__tests__/GraphQLCache-test.ts +++ b/packages/graphql-language-service-server/src/__tests__/GraphQLCache-test.ts @@ -15,12 +15,6 @@ jest.mock('@whatwg-node/fetch', () => ({ TextDecoder: global.TextDecoder, })); -// jest.mock('cross-undici-fetch', () => ({ -// fetch: require('fetch-mock').fetchHandler, -// AbortController: MockAbortController, -// TextDecoder: global.TextDecoder, -// })); - jest.mock('cross-fetch', () => ({ fetch: require('fetch-mock').fetchHandler, AbortController: MockAbortController, diff --git a/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts b/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts index c0296ebbc6b..7ba9c06bb77 100644 --- a/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts +++ b/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts @@ -63,7 +63,7 @@ describe('MessageProcessor', () => { beforeEach(async () => { const gqlConfig = await loadConfig({ rootDir: __dirname, extensions: [] }); // loadConfig.mockRestore(); - const workspaceUri = pathToFileURL(__dirname).toString(); + const workspaceUri = pathToFileURL('.').toString(); messageProcessor._sortedWorkspaceUris = [workspaceUri]; messageProcessor._processors = new Map([ @@ -337,107 +337,38 @@ describe('MessageProcessor', () => { const mockReadFileSync: jest.Mock = jest.requireMock('fs').readFileSync; beforeEach(() => { - workspaceMessageProcessor._projectCacheStatuses = new Map(); mockReadFileSync.mockReturnValue(''); - // workspaceMessageProcessor._loadAndCacheProjectConfig = jest.fn(); - workspaceMessageProcessor._cacheSchemaFilesForProject = jest.fn(); + workspaceMessageProcessor._updateGraphQLConfig = jest.fn(); }); it('updates config for standard config filename changes', async () => { - workspaceMessageProcessor._initializeConfig = jest.fn(); await messageProcessor.handleDidOpenOrSaveNotification({ textDocument: { - uri: `${pathToFileURL(__dirname)}/.graphql.config.js`, + uri: `${pathToFileURL('.')}/.graphql.config.js`, languageId: 'js', version: 0, text: '', }, }); - expect(workspaceMessageProcessor._initializeConfig).toHaveBeenCalled(); - expect( - workspaceMessageProcessor._cacheSchemaFilesForProject, - ).toHaveBeenCalled(); - }); - - it('does not rebuild project cache on second change to a file', async () => { - workspaceMessageProcessor._initializeConfig = jest.fn(); - await messageProcessor.handleDidOpenOrSaveNotification({ - textDocument: { - uri: `${pathToFileURL(__dirname)}/test.graphql`, - languageId: 'graphql', - version: 0, - text: '', - }, - }); - - expect(workspaceMessageProcessor._initializeConfig).toHaveBeenCalledTimes( - 0, - ); - expect( - workspaceMessageProcessor._cacheSchemaFilesForProject, - ).toHaveBeenCalledTimes(1); - - // make a 2nd change - await messageProcessor.handleDidOpenOrSaveNotification({ - textDocument: { - uri: `${pathToFileURL(__dirname)}/test.graphql`, - languageId: 'graphql', - version: 1, - text: '{ id }', - }, - }); - - expect(workspaceMessageProcessor._initializeConfig).toHaveBeenCalledTimes( - 0, - ); - expect( - workspaceMessageProcessor._cacheSchemaFilesForProject, - ).toHaveBeenCalledTimes(1); - - // make a 3rd change - await messageProcessor.handleDidOpenOrSaveNotification({ - textDocument: { - uri: `${pathToFileURL(__dirname)}/test.graphql`, - languageId: 'graphql', - version: 2, - text: '{ id title }', - }, - }); - - expect(workspaceMessageProcessor._initializeConfig).toHaveBeenCalledTimes( - 0, - ); - expect( - workspaceMessageProcessor._cacheSchemaFilesForProject, - ).toHaveBeenCalledTimes(1); + expect(workspaceMessageProcessor._updateGraphQLConfig).toHaveBeenCalled(); }); it('updates config for custom config filename changes', async () => { const customConfigName = 'custom-config-name.yml'; - - const newSettings = { + workspaceMessageProcessor._settings = { load: { fileName: customConfigName }, }; - workspaceMessageProcessor._settings = newSettings; - workspaceMessageProcessor._initializeConfig = jest.fn(); - - await messageProcessor.handleDidChangeConfiguration({ - settings: [newSettings], - }); - await messageProcessor.handleDidOpenOrSaveNotification({ textDocument: { - uri: `${pathToFileURL(__dirname)}/${customConfigName}`, + uri: `${pathToFileURL('.')}/${customConfigName}`, languageId: 'js', version: 0, text: '', }, }); - expect(workspaceMessageProcessor._initializeConfig).toHaveBeenCalled(); - expect( - workspaceMessageProcessor._cacheSchemaFilesForProject, - ).toHaveBeenCalled(); + + expect(workspaceMessageProcessor._updateGraphQLConfig).toHaveBeenCalled(); }); it('handles config requests with no config', async () => { @@ -447,18 +378,18 @@ describe('MessageProcessor', () => { settings: [], }); + expect(workspaceMessageProcessor._updateGraphQLConfig).toHaveBeenCalled(); + await messageProcessor.handleDidOpenOrSaveNotification({ textDocument: { - uri: `${pathToFileURL(__dirname)}/.graphql.config.js`, + uri: `${pathToFileURL('.')}/.graphql.config.js`, languageId: 'js', version: 0, text: '', }, }); - expect(workspaceMessageProcessor._initializeConfig).toHaveBeenCalled(); - expect( - workspaceMessageProcessor._cacheSchemaFilesForProject, - ).toHaveBeenCalled(); + + expect(workspaceMessageProcessor._updateGraphQLConfig).toHaveBeenCalled(); }); }); @@ -811,20 +742,20 @@ query Test { beforeEach(() => { mockReadFileSync.mockReturnValue(''); - messageProcessor._loadConfigOptions = jest.fn(); + messageProcessor._updateGraphQLConfig = jest.fn(); }); it('skips config updates for normal file changes', async () => { await messageProcessor.handleWatchedFilesChangedNotification({ changes: [ { - uri: `${queryPathUri}/test.graphql`, + uri: `${pathToFileURL('.')}/foo.graphql`, type: FileChangeType.Changed, }, ], }); - expect(messageProcessor._loadConfigOptions).not.toBeCalled(); + expect(messageProcessor._updateGraphQLConfig).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/graphql-language-service-server/src/__tests__/custom-config-name.yml b/packages/graphql-language-service-server/src/__tests__/custom-config-name.yml deleted file mode 100644 index e69de29bb2d..00000000000