Skip to content

Commit 1910049

Browse files
acaoacao
committed
fix embedded fragment definition offset bug!
1 parent 128ac4a commit 1910049

File tree

10 files changed

+126
-25
lines changed

10 files changed

+126
-25
lines changed

‎jest.config.base.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ module.exports = (dir, env = 'jsdom') => {
3333
// because of the svelte compiler's export patterns i guess?
3434
'svelte/compiler': `${__dirname}/node_modules/svelte/compiler.cjs`,
3535
},
36-
testMatch: ['**/*[-.](test|spec).[jt]s?(x)', '!**/cypress/**'],
36+
testMatch: ['**/*[-.](spec|test).[jt]s?(x)', '!**/cypress/**'],
3737
testEnvironment: env,
3838
testPathIgnorePatterns: ['node_modules', 'dist', 'cypress'],
3939
collectCoverageFrom: ['**/src/**/*.{js,jsx,ts,tsx}'],

‎jest.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
module.exports = {
2-
// ...require('./jest.config.base.js')(__dirname),
32
projects: ['<rootDir>/packages/*/jest.config.js'],
43
};

‎package.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
"cypress-open": "yarn workspace graphiql cypress-open",
5050
"dev-graphiql": "yarn workspace graphiql dev",
5151
"e2e": "yarn run e2e:build && yarn workspace graphiql e2e",
52-
"e2e:server": "yarn workspace graphiql e2e-server",
5352
"e2e:build": "WEBPACK_SERVE=1 yarn workspace graphiql build-bundles",
5453
"eslint": "NODE_OPTIONS=--max-old-space-size=4096 eslint --max-warnings=0 --ignore-path .gitignore --cache .",
5554
"format": "yarn eslint --fix && yarn pretty",
@@ -75,8 +74,6 @@
7574
"test:ci": "yarn build && jest --coverage && yarn vitest",
7675
"test:coverage": "yarn jest --coverage",
7776
"test:watch": "yarn jest --watch",
78-
"test:spec": "TEST_ENV=spec yarn jest --testPathIgnorePatterns test.ts",
79-
"test:unit": "yarn jest --testPathIgnorePatterns spec.ts",
8077
"tsc": "tsc --build",
8178
"vitest": "yarn wsrun -p -m test",
8279
"wsrun:noexamples": "wsrun --exclude-missing --exclude example-monaco-graphql-react-vite --exclude example-monaco-graphql-nextjs --exclude example-monaco-graphql-webpack --exclude example-graphiql-webpack"

‎packages/graphql-language-service-server/src/GraphQLCache.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -646,9 +646,9 @@ export class GraphQLCache implements GraphQLCacheInterface {
646646
schema = this._extendSchema(schema, schemaPath, schemaCacheKey);
647647
}
648648

649-
// if (schemaCacheKey) {
650-
// this._schemaMap.set(schemaCacheKey, schema);
651-
// }
649+
if (schemaCacheKey) {
650+
this._schemaMap.set(schemaCacheKey, schema);
651+
}
652652
return schema;
653653
};
654654

‎packages/graphql-language-service-server/src/MessageProcessor.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -672,8 +672,10 @@ export class MessageProcessor {
672672

673673
const text = await readFile(URI.parse(uri).fsPath, 'utf-8');
674674
const contents = this._parser(text, uri);
675+
const cachedDocument = this._textDocumentCache.get(uri);
676+
const version = cachedDocument ? cachedDocument.version++ : 0;
675677
await this._invalidateCache(
676-
{ uri, version: 0 },
678+
{ uri, version },
677679
URI.parse(uri).fsPath,
678680
contents,
679681
);
@@ -796,10 +798,17 @@ export class MessageProcessor {
796798
if (parentRange && res.name) {
797799
const isInline = inlineFragments.includes(res.name);
798800
const isEmbedded = DEFAULT_SUPPORTED_EXTENSIONS.includes(
799-
path.extname(textDocument.uri) as SupportedExtensionsEnum,
801+
path.extname(res.path) as SupportedExtensionsEnum,
800802
);
801-
if (isInline && isEmbedded) {
802-
const vOffset = parentRange.start.line;
803+
804+
if (isEmbedded || isInline) {
805+
const cachedDoc = this._getCachedDocument(
806+
URI.parse(res.path).toString(),
807+
);
808+
const vOffset = isEmbedded
809+
? cachedDoc?.contents[0].range?.start.line ?? 0
810+
: parentRange.start.line;
811+
803812
defRange.setStart(
804813
(defRange.start.line += vOffset),
805814
defRange.start.character,

‎packages/graphql-language-service-server/src/__tests__/MessageProcessor.spec.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ describe('project with simple config and graphql files', () => {
220220
['fragments.graphql', 'fragment T on Test {\n isTest \n}'],
221221
[
222222
'graphql.config.json',
223-
'{ "schema": "http://localhost:3100/graphql", "documents": "./**.graphql" }',
223+
'{ "schema": "http://localhost:3100/graphql", "documents": "./**" }',
224224
],
225225
],
226226
});
@@ -270,17 +270,11 @@ describe('project with simple config and graphql files', () => {
270270
},
271271
});
272272

273-
// TODO: super weird, the type definition cache isn't built until _after_ the first definitions request (for that file?)...
274-
// this may be a bug just on init, or perhaps every definitions request is outdated???
275-
// local schema file should be used for definitions
276-
277273
const typeDefinitions = await project.lsp.handleDefinitionRequest({
278274
textDocument: { uri: project.uri('fragments.graphql') },
279275
position: { character: 15, line: 0 },
280276
});
281277

282-
// TODO: these should return a type definition from the schema
283-
//
284278
expect(typeDefinitions[0].uri).toEqual(URI.parse(genSchemaPath).toString());
285279

286280
expect(serializeRange(typeDefinitions[0].range)).toEqual({
@@ -309,5 +303,33 @@ describe('project with simple config and graphql files', () => {
309303
character: 1,
310304
},
311305
});
306+
await project.deleteFile('fragments.graphql');
307+
await project.addFile(
308+
'fragments.ts',
309+
'\n\nexport const fragment = \ngql`\n\n fragment T on Test { isTest }\n`',
310+
);
311+
312+
await project.lsp.handleWatchedFilesChangedNotification({
313+
changes: [
314+
{ uri: project.uri('fragments.ts'), type: FileChangeType.Created },
315+
],
316+
});
317+
const defsForTs = await project.lsp.handleDefinitionRequest({
318+
textDocument: { uri: project.uri('query.graphql') },
319+
position: { character: 26, line: 0 },
320+
});
321+
322+
expect(defsForTs[0].uri).toEqual(project.uri('fragments.ts'));
323+
expect(serializeRange(defsForTs[0].range)).toEqual({
324+
start: {
325+
line: 5,
326+
character: 2,
327+
},
328+
end: {
329+
// TODO! line is wrong, it expects 1 for some reason probably in the LanguageService here
330+
line: 5,
331+
character: 31,
332+
},
333+
});
312334
});
313335
});

‎packages/graphql-language-service-server/src/__tests__/MessageProcessor.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,14 +289,18 @@ describe('MessageProcessor', () => {
289289
it('runs document symbol requests when not initialized', async () => {
290290
const test = {
291291
textDocument: {
292-
uri: `${queryPathUri}/test5.graphql`,
292+
uri: `${queryPathUri}/test3.graphql`,
293293
version: 0,
294294
},
295295
};
296296
messageProcessor._isInitialized = false;
297297
const result = await messageProcessor.handleDocumentSymbolRequest(test);
298298
expect(result).toEqual([]);
299299
messageProcessor._isInitialized = true;
300+
const nextResult = await messageProcessor.handleDocumentSymbolRequest(test);
301+
expect(nextResult[0].location.uri).toContain('test3.graphql');
302+
expect(nextResult[0].name).toEqual('item');
303+
expect(nextResult.length).toEqual(1);
300304
});
301305

302306
it('properly changes the file cache with the didChange handler', async () => {
@@ -335,11 +339,13 @@ describe('MessageProcessor', () => {
335339
});
336340

337341
it('does not crash on null value returned in response to workspace configuration', async () => {
342+
// for some reason this is needed? can't be a good thing... must have done something to cause a performance hit on
343+
// loading config schema..
344+
jest.setTimeout(10000);
338345
const previousConfigurationValue = getConfigurationReturnValue;
339346
getConfigurationReturnValue = null;
340-
await expect(
341-
messageProcessor.handleDidChangeConfiguration(),
342-
).resolves.toStrictEqual({});
347+
const result = await messageProcessor.handleDidChangeConfiguration();
348+
expect(result).toEqual({});
343349
getConfigurationReturnValue = previousConfigurationValue;
344350
});
345351

‎packages/graphql-language-service-server/src/__tests__/__utils__/MockProject.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import mockfs from 'mock-fs';
22
import { MessageProcessor } from '../../MessageProcessor';
33
import { Logger as VSCodeLogger } from 'vscode-jsonrpc';
44
import { URI } from 'vscode-uri';
5+
import { FileChangeType } from 'vscode-languageserver';
6+
import { FileChangeTypeKind } from 'graphql-language-service';
57

68
export type MockFile = [filename: string, text: string];
79

@@ -104,6 +106,74 @@ export class MockProject {
104106
this.fileCache.set(filename, text);
105107
this.mockFiles();
106108
}
109+
async addFile(filename: string, text: string) {
110+
this.fileCache.set(filename, text);
111+
this.mockFiles();
112+
await this.lsp.handleDidChangeNotification({
113+
contentChanges: [
114+
{
115+
type: FileChangeTypeKind.Created,
116+
text,
117+
},
118+
],
119+
textDocument: {
120+
uri: this.uri(filename),
121+
version: 2,
122+
},
123+
});
124+
}
125+
async changeWatchedFile(filename: string, text: string) {
126+
this.changeFile(filename, text);
127+
await this.lsp.handleWatchedFilesChangedNotification({
128+
changes: [
129+
{
130+
uri: this.uri(filename),
131+
type: FileChangeType.Changed,
132+
},
133+
],
134+
});
135+
}
136+
async saveOpenFile(filename: string, text: string) {
137+
this.changeFile(filename, text);
138+
await this.lsp.handleDidOpenOrSaveNotification({
139+
textDocument: {
140+
uri: this.uri(filename),
141+
version: 2,
142+
text,
143+
},
144+
});
145+
}
146+
async addWatchedFile(filename: string, text: string) {
147+
this.changeFile(filename, text);
148+
await this.lsp.handleDidChangeNotification({
149+
contentChanges: [
150+
{
151+
type: FileChangeTypeKind.Created,
152+
text,
153+
},
154+
],
155+
textDocument: {
156+
uri: this.uri(filename),
157+
version: 2,
158+
},
159+
});
160+
}
161+
async deleteFile(filename: string) {
162+
this.fileCache.delete(filename);
163+
this.mockFiles();
164+
await this.lsp.handleDidChangeNotification({
165+
contentChanges: [
166+
{
167+
type: FileChangeTypeKind.Deleted,
168+
text: '',
169+
},
170+
],
171+
textDocument: {
172+
uri: this.uri(filename),
173+
version: 2,
174+
},
175+
});
176+
}
107177
get lsp() {
108178
return this.messageProcessor;
109179
}

‎packages/graphql-language-service-server/src/__tests__/__utils__/runServer.js

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎packages/graphql-language-service/src/interface/getDefinition.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ export async function getDefinitionQueryResultForFragmentSpread(
124124
({ filePath, content, definition }) =>
125125
getDefinitionForFragmentDefinition(filePath || '', content, definition),
126126
);
127-
128127
return {
129128
definitions,
130129
queryRange: definitions.map(_ => getRange(text, fragment)),

0 commit comments

Comments
 (0)