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

fix: lsp server paths for windows, special chars, fragment/object cache #2072

Merged
merged 3 commits into from
Nov 30, 2021

Conversation

acao
Copy link
Member

@acao acao commented Nov 30, 2021

fixing windows and unicode-containing file paths

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! it was url-encoding these characters which is not helping anyone haha

fixing object and fragment definition cache across OSes

also, when switching back from fileURLToPath, we left one case in GraphQLCache which may impact how the fragment and object type cache are being loaded with a global file glob. this existing feature has led users to expect fragments and objects to load magically from config without configuration and we will need to change it, but for now we will keep the magic glob from rootDir. this is set from the graphql config rootDir so if that is overridden, it breaks how this cache works. we need in the future for users to specify the files or globs where fragments or object type definitions are contained.

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 and I think also #2071

@changeset-bot
Copy link

changeset-bot bot commented Nov 30, 2021

🦋 Changeset detected

Latest commit: 4a3bf20

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
graphql-language-service-server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@acao acao added the lsp-server graphql-language-service-server label Nov 30, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 30, 2021

The latest changes of this PR are available as canary in npm (based on the declared changesets):

@github-actions
Copy link
Contributor

github-actions bot commented Nov 30, 2021

@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview November 30, 2021 18:19 Inactive
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
@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview November 30, 2021 18:22 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Nov 30, 2021

@github-actions github-actions bot temporarily deployed to graphiql-cdn-preview November 30, 2021 18:23 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Nov 30, 2021

@github-actions github-actions bot temporarily deployed to monaco-graphql-webpack-preview November 30, 2021 18:23 Inactive
@acao acao changed the title fix: lsp server paths for windows, special chars fix: lsp server paths for windows, special chars, fragment/object cache Nov 30, 2021
@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview November 30, 2021 18:29 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-cdn-preview November 30, 2021 18:29 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql-webpack-preview November 30, 2021 18:30 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview November 30, 2021 18:52 Inactive
@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #2072 (4a3bf20) into main (2d91916) will decrease coverage by 2.35%.
The diff coverage is 63.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2072      +/-   ##
==========================================
- Coverage   65.70%   63.34%   -2.36%     
==========================================
  Files          85       94       +9     
  Lines        5106     5443     +337     
  Branches     1631     1715      +84     
==========================================
+ Hits         3355     3448      +93     
- Misses       1747     1990     +243     
- Partials        4        5       +1     
Impacted Files Coverage Δ
packages/codemirror-graphql/src/hint.ts 94.73% <ø> (ø)
packages/codemirror-graphql/src/lint.ts 100.00% <ø> (ø)
packages/codemirror-graphql/src/results/mode.ts 47.05% <ø> (ø)
...kages/codemirror-graphql/src/utils/forEachState.ts 100.00% <ø> (ø)
...ckages/codemirror-graphql/src/utils/mode-indent.ts 0.00% <0.00%> (ø)
packages/codemirror-graphql/src/variables/hint.ts 89.70% <ø> (ø)
packages/codemirror-graphql/src/variables/mode.ts 79.48% <ø> (ø)
packages/graphiql/src/components/QueryEditor.tsx 63.96% <ø> (ø)
...graphiql/src/components/__tests__/ExampleSchema.ts 100.00% <ø> (ø)
packages/graphiql/src/utility/fillLeafs.ts 5.33% <ø> (ø)
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 503c376...4a3bf20. Read the comment docs.

@github-actions github-actions bot temporarily deployed to graphiql-cdn-preview November 30, 2021 18:52 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql-webpack-preview November 30, 2021 18:53 Inactive
@acao acao merged commit c423619 into main Nov 30, 2021
@acao acao deleted the fix/server-file-paths branch November 30, 2021 19:21
@github-actions github-actions bot mentioned this pull request Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp-server graphql-language-service-server
Projects
None yet
1 participant