Skip to content

Commit aa6dbbb

Browse files
authored
LSP Rewrite: fix many bugs, more LSP test coverage! (#3521)
see: changelog entries
1 parent 03ab3a6 commit aa6dbbb

File tree

77 files changed

+5206
-2392
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+5206
-2392
lines changed

.changeset/rotten-seahorses-fry.md

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
'graphql-language-service-server': minor
3+
'vscode-graphql': minor
4+
'graphql-language-service-server-cli': minor
5+
---
6+
7+
Fix many schema and fragment lifecycle issues, not all of them, but many related to cacheing.
8+
Note: this makes `cacheSchemaForLookup` enabled by default again for schema first contexts.
9+
10+
This fixes multiple cacheing bugs, upon addomg some in-depth integration test coverage for the LSP server.
11+
It also solves several bugs regarding loading config types, and properly restarts the server and invalidates schema when there are config changes.
12+
13+
### Bugfix Summary
14+
15+
- configurable polling updates for network and other code first schema configuration, set to a 30s interval by default. powered by `schemaCacheTTL` which can be configured in the IDE settings (vscode, nvim) or in the graphql config file. (1)
16+
- jump to definition in embedded files offset bug, for both fragments and code files with SDL strings
17+
- cache invalidation for fragments (fragment lookup/autcoomplete data is more accurate, but incomplete/invalid fragments still do not autocomplete or validate, and remember fragment options always filter/validate by the `on` type!)
18+
- schema cache invalidation for schema files - schema updates as you change the SDL files, and the generated file for code first by the `schemaCacheTTL` setting
19+
- schema definition lookups & autocomplete crossing over into the wrong project
20+
21+
**Notes**
22+
23+
1. If possible, configuring for your locally running framework or a schema registry client to handle schema updates and output to a `schema.graphql` or `introspection.json` will always provide a better experience. many graphql frameworks have this built in! Otherwise, we must use this new lazy polling approach if you provide a url schema (this includes both introspection URLs and remote file URLs, and the combination of these).
24+
25+
### Known Bugs Fixed
26+
27+
- #3318
28+
- #2357
29+
- #3469
30+
- #2422
31+
- #2820
32+
- many more!
33+
34+
### Test Improvements
35+
36+
- new, high level integration spec suite for the LSP with a matching test utility
37+
- more unit test coverage
38+
- **total increased test coverage of about 25% in the LSP server codebase.**
39+
- many "happy paths" covered for both schema and code first contexts
40+
- many bugs revealed (and their source)
41+
42+
### What's next?
43+
44+
Another stage of the rewrite is already almost ready. This will fix even more bugs and improve memory usage, eliminate redundant parsing and ensure that graphql config's loaders do _all_ of the parsing and heavy lifting, thus honoring all the configs as well. It also significantly reduces the code complexity.
45+
46+
There is also a plan to match Relay LSP's lookup config for either IDE (vscode, nvm, etc) settings as they provide, or by loading modules into your `graphql-config`!

.changeset/silly-yaks-bathe.md

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'graphql-language-service': patch
3+
'graphql-language-service-server': patch
4+
'graphql-language-service-server-cli': patch
5+
'codemirror-graphql': patch
6+
'cm6-graphql': patch
7+
'monaco-graphql': patch
8+
'vscode-graphql': patch
9+
---
10+
11+
Fixes several issues with Type System (SDL) completion across the ecosystem:
12+
13+
- restores completion for object and input type fields when the document context is not detectable or parseable
14+
- correct top-level completions for either of the unknown, type system or executable definitions. this leads to mixed top level completions when the document is unparseable, but now you are not seemingly restricted to only executable top level definitions
15+
- `.graphqls` ad-hoc standard functionality remains, but is not required, as it is not part of the official spec, and the spec also allows mixed mode documents in theory, and this concept is required when the type is unknown

.changeset/wet-toes-mate.md

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
'graphql-language-service-server': minor
3+
'vscode-graphql': patch
4+
---
5+
6+
Introduce `locateCommand` based on Relay LSP `pathToLocateCommand`:
7+
8+
Now with `<graphql config>.extensions.languageService.locateCommand`, you can specify either the [existing signature](https://marketplace.visualstudio.com/items?itemName=meta.relay#relay.pathtolocatecommand-default-null) for relay, with the same callback parameters and return signature.
9+
10+
Relay LSP currently supports `Type` and `Type.field` for the 2nd argument. Ours also returns `Type.field(argument)` as a point of reference. It works with object types, input types, fragments, executable definitions and their fields, and should work for directive definitions as well.
11+
12+
In the case of unnamed types such as fragment spreads, they return the name of the implemented type currently, but I'm curious what users prefer here. I assumed that some people may want to not be limited to only using this for SDL type definition lookups. Also look soon to see `locateCommand` support added for symbols, outline, and coming references and implementations.
13+
14+
The module at the path you specify in relay LSP for `pathToLocateCommand` should work as such
15+
16+
```ts
17+
// import it
18+
import { locateCommand } from './graphql/tooling/lsp/locate.js';
19+
export default {
20+
languageService: {
21+
locateCommand,
22+
},
23+
24+
projects: {
25+
a: {
26+
schema: 'https://localhost:8000/graphql',
27+
documents: './a/**/*.{ts,tsx,jsx,js,graphql}',
28+
},
29+
b: {
30+
schema: './schema/ascode.ts',
31+
documents: './b/**/*.{ts,tsx,jsx,js,graphql}',
32+
},
33+
},
34+
};
35+
```
36+
37+
```ts
38+
// or define it inline
39+
40+
import { type LocateCommand } from 'graphql-language-service-server';
41+
42+
// relay LSP style
43+
const languageCommand = (projectName: string, typePath: string) => {
44+
const { path, startLine, endLine } = ourLookupUtility(projectName, typePath);
45+
return `${path}:${startLine}:${endLine}`;
46+
};
47+
48+
// an example with our alternative return signature
49+
const languageCommand: LocateCommand = (projectName, typePath, info) => {
50+
// pass more info, such as GraphQLType with the ast node. info.project is also available if you need it
51+
const { path, range } = ourLookupUtility(
52+
projectName,
53+
typePath,
54+
info.type.node,
55+
);
56+
return { uri: path, range }; // range.start.line/range.end.line
57+
};
58+
59+
export default {
60+
languageService: {
61+
locateCommand,
62+
},
63+
schema: 'https://localhost:8000/graphql',
64+
documents: './**/*.{ts,tsx,jsx,js,graphql}',
65+
};
66+
```
67+
68+
Passing a string as a module path to resolve is coming in a follow-up release. Then it can be used with `.yml`, `.toml`, `.json`, `package.json#graphql`, etc
69+
70+
For now this was a quick baseline for a feature asked for in multiple channels!
71+
72+
Let us know how this works, and about any other interoperability improvements between our graphql LSP and other language servers (relay, intellij, etc) used by you and colleauges in your engineering organisations. We are trying our best to keep up with the awesome innovations they have 👀!

.github/workflows/pr.yml

+4-18
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ jobs:
1313
uses: actions/checkout@v3
1414
- uses: actions/setup-node@v3
1515
with:
16-
node-version: 16
1716
cache: yarn
1817
- name: Cache node modules
1918
id: cache-modules
@@ -37,8 +36,7 @@ jobs:
3736
steps:
3837
- uses: actions/checkout@v3
3938
- uses: actions/setup-node@v3
40-
with:
41-
node-version: 16
39+
4240
- id: cache-modules
4341
uses: actions/cache@v3
4442
with:
@@ -59,8 +57,6 @@ jobs:
5957
steps:
6058
- uses: actions/checkout@v3
6159
- uses: actions/setup-node@v3
62-
with:
63-
node-version: 16
6460
- id: cache-modules
6561
uses: actions/cache@v3
6662
with:
@@ -76,8 +72,6 @@ jobs:
7672
steps:
7773
- uses: actions/checkout@v3
7874
- uses: actions/setup-node@v3
79-
with:
80-
node-version: 16
8175
- id: cache-modules
8276
uses: actions/cache@v3
8377
with:
@@ -87,14 +81,12 @@ jobs:
8781
- run: yarn pretty-check
8882

8983
jest:
90-
name: Jest Unit Tests
84+
name: Jest Unit & Integration Tests
9185
runs-on: ubuntu-latest
9286
needs: [install]
9387
steps:
9488
- uses: actions/checkout@v3
9589
- uses: actions/setup-node@v3
96-
with:
97-
node-version: 16
9890
- id: cache-modules
9991
uses: actions/cache@v3
10092
with:
@@ -117,8 +109,7 @@ jobs:
117109
steps:
118110
- uses: actions/checkout@v3
119111
- uses: actions/setup-node@v3
120-
with:
121-
node-version: 16
112+
122113
- id: cache-modules
123114
uses: actions/cache@v3
124115
with:
@@ -140,8 +131,7 @@ jobs:
140131
steps:
141132
- uses: actions/checkout@v3
142133
- uses: actions/setup-node@v3
143-
with:
144-
node-version: 16
134+
145135
- id: cache-modules
146136
uses: actions/cache@v3
147137
with:
@@ -205,8 +195,6 @@ jobs:
205195
with:
206196
fetch-depth: 0
207197
- uses: actions/setup-node@v3
208-
with:
209-
node-version: 16
210198
- id: cache-modules
211199
uses: actions/cache@v3
212200
with:
@@ -264,8 +252,6 @@ jobs:
264252
steps:
265253
- uses: actions/checkout@v3
266254
- uses: actions/setup-node@v3
267-
with:
268-
node-version: 16
269255
- id: cache-modules
270256
uses: actions/cache@v3
271257
with:

.nvmrc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
16
1+
20

cspell.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"dictionaryDefinitions": [
1010
{
1111
"name": "custom-words",
12-
"path": "./custom-words.txt",
12+
"path": "./resources/custom-words.txt",
1313
"addWords": true
1414
}
1515
],

examples/monaco-graphql-react-vite/vite.config.ts

+19
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,24 @@ export default defineConfig({
1919
},
2020
],
2121
}),
22+
watchPackages(['monaco-graphql', 'graphql-language-service']),
2223
],
2324
});
25+
26+
function watchPackages(packageNames: string[]) {
27+
let isWatching = false;
28+
29+
return {
30+
name: 'vite-plugin-watch-packages',
31+
32+
buildStart() {
33+
if (!isWatching) {
34+
for (const packageName of packageNames) {
35+
this.addWatchFile(require.resolve(packageName));
36+
}
37+
38+
isWatching = true;
39+
}
40+
},
41+
};
42+
}

jest.config.base.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module.exports = (dir, env = 'jsdom') => {
4343
'node_modules',
4444
'__tests__',
4545
'resources',
46-
'test',
46+
4747
'examples',
4848
'.d.ts',
4949
'types.ts',

package.json

+4-4
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
"lint-staged": {
1616
"*.{js,ts,jsx,tsx}": [
1717
"eslint --cache --fix",
18-
"prettier --write --ignore-path .eslintignore",
18+
"prettier --write --ignore-path .eslintignore --ignore-path resources/prettierignore",
1919
"jest --passWithNoTests",
2020
"yarn lint-cspell"
2121
],
2222
"*.{md,html,json,css}": [
23-
"prettier --write --ignore-path .eslintignore",
23+
"prettier --write --ignore-path .eslintignore --ignore-path resources/prettierignore",
2424
"yarn lint-cspell"
2525
]
2626
},
@@ -43,7 +43,7 @@
4343
"build:watch": "yarn tsc --watch",
4444
"build-demo": "wsrun -m build-demo",
4545
"watch": "yarn build:watch",
46-
"watch-vscode": "yarn workspace vscode-graphql compile",
46+
"watch-vscode": "yarn tsc && yarn workspace vscode-graphql compile",
4747
"watch-vscode-exec": "yarn workspace vscode-graphql-execution compile",
4848
"check": "yarn tsc --noEmit",
4949
"cypress-open": "yarn workspace graphiql cypress-open",
@@ -62,7 +62,7 @@
6262
"prepublishOnly": "./scripts/prepublish.sh",
6363
"postbuild": "wsrun --exclude-missing postbuild",
6464
"pretty": "yarn pretty-check --write",
65-
"pretty-check": "prettier --cache --check --ignore-path .gitignore --ignore-path .eslintignore .",
65+
"pretty-check": "prettier --cache --check --ignore-path .gitignore --ignore-path resources/prettierignore --ignore-path .eslintignore .",
6666
"ci:version": "yarn changeset version && yarn build && yarn format",
6767
"release": "yarn build && yarn build-bundles && (wsrun release --exclude-missing --serial --recursive --changedSince main -- || true) && yarn changeset publish",
6868
"release:canary": "(node scripts/canary-release.js && yarn build-bundles && yarn changeset publish --tag canary) || echo Skipping Canary...",

packages/cm6-graphql/src/completions.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,14 @@ export const completion = graphqlLanguage.data.of({
2626
}
2727
const val = ctx.state.doc.toString();
2828
const pos = offsetToPos(ctx.state.doc, ctx.pos);
29-
const results = getAutocompleteSuggestions(schema, val, pos);
29+
const results = getAutocompleteSuggestions(
30+
schema,
31+
val,
32+
pos,
33+
undefined,
34+
undefined,
35+
opts?.autocompleteOptions,
36+
);
3037

3138
if (results.length === 0) {
3239
return null;

packages/cm6-graphql/src/interfaces.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { Completion, CompletionContext } from '@codemirror/autocomplete';
22
import { EditorView } from '@codemirror/view';
33
import { GraphQLSchema } from 'graphql';
4-
import { ContextToken, CompletionItem } from 'graphql-language-service';
4+
import {
5+
ContextToken,
6+
CompletionItem,
7+
AutocompleteSuggestionOptions,
8+
} from 'graphql-language-service';
59
import { Position } from './helpers';
610
export interface GqlExtensionsOptions {
711
showErrorOnInvalidSchema?: boolean;
@@ -18,4 +22,5 @@ export interface GqlExtensionsOptions {
1822
ctx: CompletionContext,
1923
item: Completion,
2024
) => Node | Promise<Node | null> | null;
25+
autocompleteOptions?: AutocompleteSuggestionOptions;
2126
}

0 commit comments

Comments
 (0)