Skip to content

Commit

Permalink
fix: lsp server paths for windows, special chars, fragment/object cac…
Browse files Browse the repository at this point in the history
…he (#2072)
  • Loading branch information
acao committed Nov 30, 2021
1 parent 503c376 commit c423619
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 64 deletions.
17 changes: 17 additions & 0 deletions .changeset/fifty-actors-sleep.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 3 additions & 1 deletion packages/graphql-language-service-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions packages/graphql-language-service-server/src/GraphQLCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { parseDocument } from './parseDocument';
import stringToHash from './stringToHash';
import glob from 'glob';
import { LoadConfigOptions } from './types';
import { fileURLToPath, pathToFileURL } from 'url';
import { URI } from 'vscode-uri';

// Maximum files to read when processing GraphQL files.
const MAX_READS = 200;
Expand Down Expand Up @@ -106,7 +106,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 (
Expand Down Expand Up @@ -365,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,
};
Expand Down Expand Up @@ -776,7 +776,7 @@ export class GraphQLCache implements GraphQLCacheInterface {
*/
promiseToReadGraphQLFile = (filePath: Uri): Promise<GraphQLFileInfo> => {
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;
Expand Down
44 changes: 21 additions & 23 deletions packages/graphql-language-service-server/src/MessageProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@

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';
import {
CachedContent,
Uri,
Expand Down Expand Up @@ -131,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 &&
Expand Down Expand Up @@ -225,11 +225,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) {
Expand All @@ -248,6 +247,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 {
Expand Down Expand Up @@ -283,6 +283,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;
Expand Down Expand Up @@ -567,25 +579,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);
Expand Down Expand Up @@ -831,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) {
Expand All @@ -859,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);
}
Expand Down Expand Up @@ -1014,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
});
});
});
31 changes: 24 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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" "*"

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down

2 comments on commit c423619

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.