From 04a2552edb9d09eac9256c720bb2fd49ab4184c8 Mon Sep 17 00:00:00 2001 From: Rikki Schulte Date: Tue, 30 Nov 2021 19:15:25 +0100 Subject: [PATCH 1/3] fix: lsp server paths for windows, special chars using the standard `vscode-uri` package, this should fix the parsing of file URIs by `graphql-language-service-server` in cases such as: - windows without WSL - special/unicode characters in filenames - likely other cases previously we were using the old approach of `URL(uri).pathname` which was not working! this should fix issues with object and fragment type completion as well I think --- .../package.json | 4 ++- .../src/GraphQLCache.ts | 7 ++-- .../src/MessageProcessor.ts | 33 ++++++++++--------- yarn.lock | 31 +++++++++++++---- 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/packages/graphql-language-service-server/package.json b/packages/graphql-language-service-server/package.json index 470207885f7..9fcbf65a812 100644 --- a/packages/graphql-language-service-server/package.json +++ b/packages/graphql-language-service-server/package.json @@ -40,7 +40,9 @@ "nullthrows": "^1.0.0", "vscode-jsonrpc": "^5.0.1", "vscode-languageserver": "^6.1.1", - "fast-glob": "^3.2.7" + "fast-glob": "^3.2.7", + "vscode-uri": "^3.0.2", + "glob": "^7.2.0" }, "devDependencies": { "@types/mkdirp": "^1.0.1", diff --git a/packages/graphql-language-service-server/src/GraphQLCache.ts b/packages/graphql-language-service-server/src/GraphQLCache.ts index 15fd68904dd..d6b3ddee309 100644 --- a/packages/graphql-language-service-server/src/GraphQLCache.ts +++ b/packages/graphql-language-service-server/src/GraphQLCache.ts @@ -31,7 +31,8 @@ import { parseDocument } from './parseDocument'; import stringToHash from './stringToHash'; import glob from 'glob'; import { LoadConfigOptions } from './types'; -import { fileURLToPath, pathToFileURL } from 'url'; +import { pathToFileURL } from 'url'; +import { URI } from 'vscode-uri'; // Maximum files to read when processing GraphQL files. const MAX_READS = 200; @@ -106,7 +107,7 @@ export class GraphQLCache implements GraphQLCacheInterface { getGraphQLConfig = (): GraphQLConfig => this._graphQLConfig; getProjectForFile = (uri: string): GraphQLProjectConfig => { - return this._graphQLConfig.getProjectForFile(new URL(uri).pathname); + return this._graphQLConfig.getProjectForFile(URI.parse(uri).fsPath); }; getFragmentDependencies = async ( @@ -776,7 +777,7 @@ export class GraphQLCache implements GraphQLCacheInterface { */ promiseToReadGraphQLFile = (filePath: Uri): Promise => { return new Promise((resolve, reject) => - fs.readFile(fileURLToPath(filePath), 'utf8', (error, content) => { + fs.readFile(URI.parse(filePath).fsPath, 'utf8', (error, content) => { if (error) { reject(error); return; diff --git a/packages/graphql-language-service-server/src/MessageProcessor.ts b/packages/graphql-language-service-server/src/MessageProcessor.ts index feb05fccef6..a2e237894b2 100644 --- a/packages/graphql-language-service-server/src/MessageProcessor.ts +++ b/packages/graphql-language-service-server/src/MessageProcessor.ts @@ -12,6 +12,7 @@ import { readFileSync, existsSync, writeFileSync, writeFile } from 'fs'; import { pathToFileURL } from 'url'; import * as path from 'path'; import glob from 'fast-glob'; +import { URI } from 'vscode-uri'; import { CachedContent, Uri, @@ -225,11 +226,10 @@ export class MessageProcessor { }, this._settings.load), rootDir, }; - + // reload the graphql cache this._graphQLCache = await getGraphQLCache({ parser: this._parser, loadConfigOptions: this._loadConfigOptions, - config: this._graphQLConfig, }); this._languageService = new GraphQLLanguageService(this._graphQLCache); if (this._graphQLCache?.getGraphQLConfig) { @@ -248,6 +248,7 @@ export class MessageProcessor { try { if (!this._isInitialized || !this._graphQLCache) { if (!this._settings) { + // then initial call to update graphql config await this._updateGraphQLConfig(); this._isInitialized = true; } else { @@ -283,6 +284,18 @@ export class MessageProcessor { await this._invalidateCache(textDocument, uri, contents); } else { + const configMatchers = [ + 'graphql.config', + 'graphqlrc', + 'package.json', + this._settings.load.fileName, + ].filter(Boolean); + 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! const cachedDocument = this._getCachedDocument(textDocument.uri); if (cachedDocument) { contents = cachedDocument.contents; @@ -567,25 +580,13 @@ export class MessageProcessor { params.changes.map(async (change: FileEvent) => { if (!this._isInitialized || !this._graphQLCache) { throw Error('No cache available for handleWatchedFilesChanged'); - } - // update when graphql config changes! - const configMatchers = [ - 'graphql.config', - 'graphqlrc', - this._settings.load.fileName, - ].filter(Boolean); - if (configMatchers.some(v => change.uri.match(v)?.length)) { - this._logger.info('updating graphql config'); - this._updateGraphQLConfig(); - } - - if ( + } else if ( change.type === FileChangeTypeKind.Created || change.type === FileChangeTypeKind.Changed ) { const uri = change.uri; - const text = readFileSync(new URL(uri).pathname).toString(); + const text = readFileSync(URI.parse(uri).fsPath, 'utf-8'); const contents = this._parser(text, uri); await this._updateFragmentDefinition(uri, contents); diff --git a/yarn.lock b/yarn.lock index 47c646ca43f..b9401076da3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5102,10 +5102,10 @@ resolved "https://registry.yarnpkg.com/@types/minimist/-/minimist-1.2.1.tgz#283f669ff76d7b8260df8ab7a4262cc83d988256" integrity sha512-fZQQafSREFyuZcdWFAExYjBiCL7AUCdgsk80iO0q4yihYYdcIiH28CcuPTGFgLOCC8RlW49GSQxdHwZP+I7CNg== -"@types/mkdirp@^1.0.1": - version "1.0.1" - resolved "https://registry.yarnpkg.com/@types/mkdirp/-/mkdirp-1.0.1.tgz#0930b948914a78587de35458b86c907b6e98bbf6" - integrity sha512-HkGSK7CGAXncr8Qn/0VqNtExEE+PHMWb+qlR1faHMao7ng6P3tAaoWWBMdva0gL5h4zprjIO89GJOLXsMcDm1Q== +"@types/mkdirp@^1.0.`1": + version "1.0.2" + resolved "https://registry.yarnpkg.com/@types/mkdirp/-/mkdirp-1.0.2.tgz#8d0bad7aa793abe551860be1f7ae7f3198c16666" + integrity sha512-o0K1tSO0Dx5X6xlU5F1D6625FawhC3dU3iqr25lluNv/+/QIVH8RLNEiVokgIZo+mz+87w/3Mkg/VvQS+J51fQ== dependencies: "@types/node" "*" @@ -10784,6 +10784,18 @@ glob@^7.0.0, glob@^7.0.3, glob@^7.0.5, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, gl once "^1.3.0" path-is-absolute "^1.0.0" +glob@^7.2.0: + version "7.2.0" + resolved "https://registry.yarnpkg.com/glob/-/glob-7.2.0.tgz#d15535af7732e02e948f4c41628bd910293f6023" + integrity sha512-lmLf6gtyrPq8tTjSmrO94wBeQbFR3HbLHbuyD69wuyQkImp2hWqMGB47OX65FBkPffO641IP9jWa1z4ivqG26Q== + dependencies: + fs.realpath "^1.0.0" + inflight "^1.0.4" + inherits "2" + minimatch "^3.0.4" + once "^1.3.0" + path-is-absolute "^1.0.0" + global-dirs@^2.0.1: version "2.1.0" resolved "https://registry.yarnpkg.com/global-dirs/-/global-dirs-2.1.0.tgz#e9046a49c806ff04d6c1825e196c8f0091e8df4d" @@ -11003,16 +11015,16 @@ grapheme-splitter@^1.0.4: integrity sha512-bzh50DW9kTPM00T8y4o8vQg89Di9oLJVLW/KaOGIXJWP/iqCN6WKYkbNOF04vFLJhwcpYUh9ydh/+5vpOqV4YQ== "graphiql@file:packages/graphiql": - version "1.5.5" + version "1.5.7" dependencies: "@graphiql/toolkit" "^0.4.2" codemirror "^5.58.2" - codemirror-graphql "^1.2.3" + codemirror-graphql "^1.2.4" copy-to-clipboard "^3.2.0" dset "^3.1.0" entities "^2.0.0" escape-html "^1.0.3" - graphql-language-service "^3.2.3" + graphql-language-service "^3.2.4" markdown-it "^12.2.0" graphql-config@^4.1.0: @@ -19949,6 +19961,11 @@ vscode-languageserver@^6.1.1: dependencies: vscode-languageserver-protocol "^3.15.3" +vscode-uri@^3.0.2: + version "3.0.2" + resolved "https://registry.yarnpkg.com/vscode-uri/-/vscode-uri-3.0.2.tgz#ecfd1d066cb8ef4c3a208decdbab9a8c23d055d0" + integrity sha512-jkjy6pjU1fxUvI51P+gCsxg1u2n8LSt0W6KrCNQceaziKzff74GoWmjVG46KieVzybO1sttPQmYfrwSHey7GUA== + w3c-hr-time@^1.0.1, w3c-hr-time@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/w3c-hr-time/-/w3c-hr-time-1.0.2.tgz#0a89cdf5cc15822df9c360543676963e0cc308cd" From bf165004fc2da52adfb7ab43b14a1da1b1fede40 Mon Sep 17 00:00:00 2001 From: Rikki Schulte Date: Tue, 30 Nov 2021 19:26:21 +0100 Subject: [PATCH 2/3] fix file URI parsing for windows --- .changeset/fifty-actors-sleep.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .changeset/fifty-actors-sleep.md diff --git a/.changeset/fifty-actors-sleep.md b/.changeset/fifty-actors-sleep.md new file mode 100644 index 00000000000..b0cfd27c429 --- /dev/null +++ b/.changeset/fifty-actors-sleep.md @@ -0,0 +1,17 @@ +--- +'graphql-language-service-server': patch +--- + +this fixes the parsing of file URIs by `graphql-language-service-server` in cases such as: + +- windows without WSL +- special characters in filenames +- likely other cases + +previously we were using the old approach of `URL(uri).pathname` which was not working! now using the standard `vscode-uri` approach of `URI.parse(uri).fsName`. + +this should fix issues with object and fragment type completion as well I think + +also for #2066 made it so that graphql config is not loaded into the file cache unnecessarily, and that it's only loaded on editor save events rather than on file changed events + +fixes #1644 and #2066 From 4a3bf20fb7cb999ab2cf2260c2efbf213a886fd9 Mon Sep 17 00:00:00 2001 From: Rikki Schulte Date: Tue, 30 Nov 2021 19:49:18 +0100 Subject: [PATCH 3/3] remove filePathToUrl as well, fix tests --- .../src/GraphQLCache.ts | 3 +- .../src/MessageProcessor.ts | 11 ++-- .../src/__tests__/MessageProcessor-test.ts | 66 +++++++++++-------- 3 files changed, 42 insertions(+), 38 deletions(-) diff --git a/packages/graphql-language-service-server/src/GraphQLCache.ts b/packages/graphql-language-service-server/src/GraphQLCache.ts index d6b3ddee309..fc3fb2c25fb 100644 --- a/packages/graphql-language-service-server/src/GraphQLCache.ts +++ b/packages/graphql-language-service-server/src/GraphQLCache.ts @@ -31,7 +31,6 @@ import { parseDocument } from './parseDocument'; import stringToHash from './stringToHash'; import glob from 'glob'; import { LoadConfigOptions } from './types'; -import { pathToFileURL } from 'url'; import { URI } from 'vscode-uri'; // Maximum files to read when processing GraphQL files. @@ -366,7 +365,7 @@ export class GraphQLCache implements GraphQLCacheInterface { // the docs indicate that is what's there :shrug: const cacheEntry = globResult.statCache[filePath] as fs.Stats; return { - filePath: pathToFileURL(filePath).toString(), + filePath: URI.parse(filePath).toString(), mtime: Math.trunc(cacheEntry.mtime.getTime() / 1000), size: cacheEntry.size, }; diff --git a/packages/graphql-language-service-server/src/MessageProcessor.ts b/packages/graphql-language-service-server/src/MessageProcessor.ts index a2e237894b2..5c2356ef981 100644 --- a/packages/graphql-language-service-server/src/MessageProcessor.ts +++ b/packages/graphql-language-service-server/src/MessageProcessor.ts @@ -9,7 +9,6 @@ import mkdirp from 'mkdirp'; import { readFileSync, existsSync, writeFileSync, writeFile } from 'fs'; -import { pathToFileURL } from 'url'; import * as path from 'path'; import glob from 'fast-glob'; import { URI } from 'vscode-uri'; @@ -132,7 +131,7 @@ export class MessageProcessor { }; this._tmpDir = tmpDir || tmpdir(); this._tmpDirBase = path.join(this._tmpDir, 'graphql-language-service'); - this._tmpUriBase = pathToFileURL(this._tmpDirBase).toString(); + this._tmpUriBase = URI.parse(this._tmpDirBase).toString(); this._loadConfigOptions = loadConfigOptions; if ( loadConfigOptions.extensions && @@ -832,9 +831,7 @@ export class MessageProcessor { const isFileUri = existsSync(uri); let version = 1; if (isFileUri) { - const schemaUri = pathToFileURL( - path.join(project.dirpath, uri), - ).toString(); + const schemaUri = URI.parse(path.join(project.dirpath, uri)).toString(); const schemaDocument = this._getCachedDocument(schemaUri); if (schemaDocument) { @@ -860,7 +857,7 @@ export class MessageProcessor { projectTmpPath = path.join(projectTmpPath, appendPath); } if (prependWithProtocol) { - return pathToFileURL(path.resolve(projectTmpPath)).toString(); + return URI.parse(path.resolve(projectTmpPath)).toString(); } else { return path.resolve(projectTmpPath); } @@ -1015,7 +1012,7 @@ export class MessageProcessor { } // build full system URI path with protocol - const uri = pathToFileURL(filePath).toString(); + const uri = URI.parse(filePath).toString(); // i would use the already existing graphql-config AST, but there are a few reasons we can't yet const contents = this._parser(document.rawSDL, uri); 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 2a6fbb767df..471a379aff4 100644 --- a/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts +++ b/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts @@ -315,6 +315,43 @@ describe('MessageProcessor', () => { await expect(result[0].uri).toEqual(`${queryPathUri}/test3.graphql`); }); + describe('handleDidOpenOrSaveNotification', () => { + const mockReadFileSync: jest.Mock = jest.requireMock('fs').readFileSync; + + beforeEach(() => { + mockReadFileSync.mockReturnValue(''); + messageProcessor._updateGraphQLConfig = jest.fn(); + }); + it('updates config for standard config filename changes', async () => { + await messageProcessor.handleDidOpenOrSaveNotification({ + textDocument: { + uri: `${pathToFileURL('.')}/.graphql.config.js`, + languageId: 'js', + version: 0, + text: '', + }, + }); + + expect(messageProcessor._updateGraphQLConfig).toHaveBeenCalled(); + }); + + it('updates config for custom config filename changes', async () => { + const customConfigName = 'custom-config-name.yml'; + messageProcessor._settings = { load: { fileName: customConfigName } }; + + await messageProcessor.handleDidOpenOrSaveNotification({ + textDocument: { + uri: `${pathToFileURL('.')}/${customConfigName}`, + languageId: 'js', + version: 0, + text: '', + }, + }); + + expect(messageProcessor._updateGraphQLConfig).toHaveBeenCalled(); + }); + }); + it('parseDocument finds queries in tagged templates', async () => { const text = ` // @flow @@ -506,34 +543,5 @@ export function Example(arg: string) {}`; expect(messageProcessor._updateGraphQLConfig).not.toHaveBeenCalled(); }); - - it('updates config for standard config filename changes', async () => { - await messageProcessor.handleWatchedFilesChangedNotification({ - changes: [ - { - uri: `${pathToFileURL('.')}/.graphql.config`, - type: FileChangeType.Changed, - }, - ], - }); - - expect(messageProcessor._updateGraphQLConfig).toHaveBeenCalled(); - }); - - it('updates config for custom config filename changes', async () => { - const customConfigName = 'custom-config-name.yml'; - messageProcessor._settings = { load: { fileName: customConfigName } }; - - await messageProcessor.handleWatchedFilesChangedNotification({ - changes: [ - { - uri: `${pathToFileURL('.')}/${customConfigName}`, - type: FileChangeType.Changed, - }, - ], - }); - - expect(messageProcessor._updateGraphQLConfig).toHaveBeenCalled(); - }); }); });