Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSP Rewrite Step 1: fix many bugs, more LSP test coverage! #3521

Merged
merged 75 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
ce984b0
test after coverage for recent features/changes 😬
acao Jan 27, 2024
328771f
fix schema file cacheing, cache by default
acao Jan 27, 2024
f48b92e
more cleanup, glob if present
acao Jan 27, 2024
0e67831
begin seperating out specs from unit tests
acao Jan 27, 2024
f00dd1c
more unit coverage
acao Jan 27, 2024
988b43c
more unit coverage
acao Jan 27, 2024
04d2768
spelling error
acao Jan 27, 2024
28bdaca
test empty cases
acao Jan 28, 2024
cb63260
config error handling
acao Feb 1, 2024
c5c1c4b
add integration spec coverage for cacheing the schema file!
acao Feb 4, 2024
3ffd03d
really exciting spec coverage
acao Feb 4, 2024
1d735b5
more improvements and coverage
acao Feb 4, 2024
2c9049b
refactor the whole integration suite
acao Feb 5, 2024
2a32282
get set up for a local schema lifecycle
acao Feb 11, 2024
4a7501f
position job correctly
acao Feb 11, 2024
6174e92
avoid changed schema for now
acao Feb 12, 2024
0a98b0d
expose a slightly changed dev server
acao Feb 12, 2024
a8bace6
tests not running seperately
acao Feb 12, 2024
bd86e44
fix workflow deps
acao Feb 16, 2024
2f4f781
fix eslint
acao Feb 16, 2024
20ffd39
attempt to fix CI only test bug
acao Feb 16, 2024
c2565e8
codecov config
acao Feb 16, 2024
cc058a7
revert test schema change
acao Feb 16, 2024
d28296e
revert config change, restore coverage
acao Feb 18, 2024
bab7612
revert config change, restore coverage
acao Feb 18, 2024
3544cd9
cleanup
acao Feb 18, 2024
2725e0c
migrate over the wire tests to use local schema instance
acao Feb 18, 2024
a7e3ac0
test script
acao Feb 18, 2024
03ac9d5
try to fix this test
acao Feb 18, 2024
3306ad5
fix a few more things related to type cacheing
acao Feb 18, 2024
6882ef0
fix embedded fragment definition offset bug!
acao Feb 20, 2024
72563fc
spelling bug
acao Feb 20, 2024
71f1ee8
cleanup
acao Feb 20, 2024
43bbe28
fix: cleanup, potentially fix project name cache key bug?
acao Feb 27, 2024
db17831
fix: delete the unused method
acao Feb 27, 2024
1fef3e4
add comments
acao Feb 27, 2024
fb57a39
cleanup
acao Feb 27, 2024
cd97ee9
feat: lazy initialization on watched file changes
acao Mar 2, 2024
62035da
fix even MORE bugs
acao Mar 3, 2024
5902c50
fix object field completion, add tests for the missing cases
acao Mar 3, 2024
529e40b
fix log level, keep things relevant
acao Mar 3, 2024
ab06142
fix: logger tests, simple re-instantiation on settings change
acao Mar 3, 2024
cbae450
add changeset
acao Mar 3, 2024
0302187
fix: env, timeout
acao Mar 8, 2024
8bb7f4b
docs: pluck some docs improvements from the next phase
acao Mar 17, 2024
a9b46e5
fix code first schema cacheing!
acao Mar 24, 2024
d397727
fix: unused declarations
acao Apr 7, 2024
9c22964
fix: ovsx publish
acao Apr 7, 2024
fc0e497
fix: third completion context mode, changeset update
acao Apr 7, 2024
1230fb9
feat: inject __typename for template tag replacement, completion impr…
acao May 1, 2024
d5b223d
fix: cleanup
acao May 1, 2024
db58db0
fix: test file/prettier whitespace conflict, add test for inline SDL …
acao May 1, 2024
8934219
feat: add relay-lsp style locateCommand
acao May 6, 2024
6ac0086
fix: fill leafs on complete config gating, fine tuning
acao May 12, 2024
11245e2
test for field argument definition lookup
acao May 18, 2024
45faf79
coverage for fillLeafsOnComplete
acao May 18, 2024
b08cebc
more coverage?
acao May 18, 2024
97c0b2d
variable type hover test
acao May 18, 2024
3d033a4
include completion.documentation in tests
acao May 19, 2024
798d782
coverage for locateCommand
acao May 19, 2024
0bab7cb
fix cm5 hint bug
acao May 19, 2024
51aed54
finish coverage for locateCommand
acao May 19, 2024
7e32382
fix lint
acao May 19, 2024
70707b9
more low level unit tests for definitions
acao May 19, 2024
203a2c0
startServer coverage
acao May 19, 2024
298565a
fix: use fetch mock
acao May 26, 2024
3ae2c35
fix: no node16
acao May 26, 2024
496c364
fix: build issue
acao May 26, 2024
66b7bf2
changelog update
acao May 26, 2024
a9e3a18
update nvmrc of course!
acao May 26, 2024
e5ab31e
tweak to try to fix ci bash expansion
acao May 27, 2024
436608c
test config output in CI
acao May 27, 2024
bd5a13a
rename one last test...
acao May 27, 2024
dade0b5
fix prettierignore, apply everywhere prettier is used
acao May 27, 2024
f6b3e6c
fix prettierignore
acao May 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 46 additions & 0 deletions .changeset/rotten-seahorses-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
'graphql-language-service-server': minor
'vscode-graphql': minor
'graphql-language-service-server-cli': minor
---

Fix many schema and fragment lifecycle issues, not all of them, but many related to cacheing.
Note: this makes `cacheSchemaForLookup` enabled by default again for schema first contexts.

This fixes multiple cacheing bugs, upon addomg some in-depth integration test coverage for the LSP server.
It also solves several bugs regarding loading config types, and properly restarts the server and invalidates schema when there are config changes.

### Bugfix Summary

- 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)
- jump to definition in embedded files offset bug, for both fragments and code files with SDL strings
- 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!)
- 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
- schema definition lookups & autocomplete crossing over into the wrong project

**Notes**

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).

### Known Bugs Fixed

- #3318
- #2357
- #3469
- #2422
- #2820
- many more!

### Test Improvements

- new, high level integration spec suite for the LSP with a matching test utility
- more unit test coverage
- **total increased test coverage of about 25% in the LSP server codebase.**
acao marked this conversation as resolved.
Show resolved Hide resolved
- many "happy paths" covered for both schema and code first contexts
- many bugs revealed (and their source)

### What's next?

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.

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`!
15 changes: 15 additions & 0 deletions .changeset/silly-yaks-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'graphql-language-service': patch
'graphql-language-service-server': patch
'graphql-language-service-server-cli': patch
'codemirror-graphql': patch
'cm6-graphql': patch
'monaco-graphql': patch
'vscode-graphql': patch
---

Fixes several issues with Type System (SDL) completion across the ecosystem:

- restores completion for object and input type fields when the document context is not detectable or parseable
- 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
- `.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
72 changes: 72 additions & 0 deletions .changeset/wet-toes-mate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
---
'graphql-language-service-server': minor
'vscode-graphql': patch
---

Introduce `locateCommand` based on Relay LSP `pathToLocateCommand`:

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.

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.

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.

The module at the path you specify in relay LSP for `pathToLocateCommand` should work as such

```ts
// import it
import { locateCommand } from './graphql/tooling/lsp/locate.js';
export default {
languageService: {
locateCommand,
},

projects: {
a: {
schema: 'https://localhost:8000/graphql',
documents: './a/**/*.{ts,tsx,jsx,js,graphql}',
},
b: {
schema: './schema/ascode.ts',
documents: './b/**/*.{ts,tsx,jsx,js,graphql}',
},
},
};
```

```ts
// or define it inline

import { type LocateCommand } from 'graphql-language-service-server';

// relay LSP style
const languageCommand = (projectName: string, typePath: string) => {
const { path, startLine, endLine } = ourLookupUtility(projectName, typePath);
return `${path}:${startLine}:${endLine}`;
};

// an example with our alternative return signature
const languageCommand: LocateCommand = (projectName, typePath, info) => {
// pass more info, such as GraphQLType with the ast node. info.project is also available if you need it
const { path, range } = ourLookupUtility(
projectName,
typePath,
info.type.node,
);
return { uri: path, range }; // range.start.line/range.end.line
};

export default {
languageService: {
locateCommand,
},
schema: 'https://localhost:8000/graphql',
documents: './**/*.{ts,tsx,jsx,js,graphql}',
};
```

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

For now this was a quick baseline for a feature asked for in multiple channels!

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 👀!
22 changes: 4 additions & 18 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ jobs:
uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
cache: yarn
- name: Cache node modules
id: cache-modules
Expand All @@ -37,8 +36,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16

- id: cache-modules
uses: actions/cache@v3
with:
Expand All @@ -59,8 +57,6 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
- id: cache-modules
uses: actions/cache@v3
with:
Expand All @@ -76,8 +72,6 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
- id: cache-modules
uses: actions/cache@v3
with:
Expand All @@ -87,14 +81,12 @@ jobs:
- run: yarn pretty-check

jest:
name: Jest Unit Tests
name: Jest Unit & Integration Tests
runs-on: ubuntu-latest
needs: [install]
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
- id: cache-modules
uses: actions/cache@v3
with:
Expand All @@ -117,8 +109,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16

- id: cache-modules
uses: actions/cache@v3
with:
Expand All @@ -140,8 +131,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16

- id: cache-modules
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -205,8 +195,6 @@ jobs:
with:
fetch-depth: 0
- uses: actions/setup-node@v3
with:
node-version: 16
- id: cache-modules
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -264,8 +252,6 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
- id: cache-modules
uses: actions/cache@v3
with:
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
16
20
2 changes: 1 addition & 1 deletion cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"dictionaryDefinitions": [
{
"name": "custom-words",
"path": "./custom-words.txt",
"path": "./resources/custom-words.txt",
"addWords": true
}
],
Expand Down
19 changes: 19 additions & 0 deletions examples/monaco-graphql-react-vite/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,24 @@ export default defineConfig({
},
],
}),
watchPackages(['monaco-graphql', 'graphql-language-service']),
],
});

function watchPackages(packageNames: string[]) {
let isWatching = false;

return {
name: 'vite-plugin-watch-packages',

buildStart() {
if (!isWatching) {
for (const packageName of packageNames) {
this.addWatchFile(require.resolve(packageName));
}

isWatching = true;
}
},
};
}
2 changes: 1 addition & 1 deletion jest.config.base.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module.exports = (dir, env = 'jsdom') => {
'node_modules',
'__tests__',
'resources',
'test',

'examples',
'.d.ts',
'types.ts',
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
"lint-staged": {
"*.{js,ts,jsx,tsx}": [
"eslint --cache --fix",
"prettier --write --ignore-path .eslintignore",
"prettier --write --ignore-path .eslintignore --ignore-path resources/prettierignore",
"jest --passWithNoTests",
"yarn lint-cspell"
],
"*.{md,html,json,css}": [
"prettier --write --ignore-path .eslintignore",
"prettier --write --ignore-path .eslintignore --ignore-path resources/prettierignore",
"yarn lint-cspell"
]
},
Expand All @@ -43,7 +43,7 @@
"build:watch": "yarn tsc --watch",
"build-demo": "wsrun -m build-demo",
"watch": "yarn build:watch",
"watch-vscode": "yarn workspace vscode-graphql compile",
"watch-vscode": "yarn tsc && yarn workspace vscode-graphql compile",
"watch-vscode-exec": "yarn workspace vscode-graphql-execution compile",
"check": "yarn tsc --noEmit",
"cypress-open": "yarn workspace graphiql cypress-open",
Expand All @@ -62,7 +62,7 @@
"prepublishOnly": "./scripts/prepublish.sh",
"postbuild": "wsrun --exclude-missing postbuild",
"pretty": "yarn pretty-check --write",
"pretty-check": "prettier --cache --check --ignore-path .gitignore --ignore-path .eslintignore .",
"pretty-check": "prettier --cache --check --ignore-path .gitignore --ignore-path resources/prettierignore --ignore-path .eslintignore .",
"ci:version": "yarn changeset version && yarn build && yarn format",
"release": "yarn build && yarn build-bundles && (wsrun release --exclude-missing --serial --recursive --changedSince main -- || true) && yarn changeset publish",
"release:canary": "(node scripts/canary-release.js && yarn build-bundles && yarn changeset publish --tag canary) || echo Skipping Canary...",
Expand Down
9 changes: 8 additions & 1 deletion packages/cm6-graphql/src/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ export const completion = graphqlLanguage.data.of({
}
const val = ctx.state.doc.toString();
const pos = offsetToPos(ctx.state.doc, ctx.pos);
const results = getAutocompleteSuggestions(schema, val, pos);
const results = getAutocompleteSuggestions(
schema,
val,
pos,
undefined,
undefined,
opts?.autocompleteOptions,
);

if (results.length === 0) {
return null;
Expand Down
7 changes: 6 additions & 1 deletion packages/cm6-graphql/src/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { Completion, CompletionContext } from '@codemirror/autocomplete';
import { EditorView } from '@codemirror/view';
import { GraphQLSchema } from 'graphql';
import { ContextToken, CompletionItem } from 'graphql-language-service';
import {
ContextToken,
CompletionItem,
AutocompleteSuggestionOptions,
} from 'graphql-language-service';
import { Position } from './helpers';
export interface GqlExtensionsOptions {
showErrorOnInvalidSchema?: boolean;
Expand All @@ -18,4 +22,5 @@ export interface GqlExtensionsOptions {
ctx: CompletionContext,
item: Completion,
) => Node | Promise<Node | null> | null;
autocompleteOptions?: AutocompleteSuggestionOptions;
}