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

feat: completion logic (for implements & variables) #1747

Merged
merged 7 commits into from
Jan 3, 2021

Conversation

acao
Copy link
Member

@acao acao commented Dec 28, 2020

in order to introduce new language features across language server and browser ecosystems, here we combine the autocomplete logic in codemirror-graphql and in the graphql-language-service-interface.

& and implements

  • introduces feat: interfaces implementing interfaces for the LSP #1742 parsing and autocomplete logic for & syntax to codemirror-graphql and thus graphiql
  • address several edge cases present in feat: interfaces implementing interfaces for the LSP #1742 at the LSP level
  • implements logic now covers:
    • filtering out interface suggestions which are already in the implements & list
    • filtering out the interface suggestion for the name of the interface you're extending
    • show both schema interfaces and interfaces in the file
    • show interface name suggestions as you type
    • (these bugs were introduced because we decided to include interfaces which aren't part of schema yet, but are inline in the file, to avoid confusion)

completion for variables

  • because we are combining autocomplete logic, the variables completion logic used by the LSP server was causing issues in GraphiQL
  • to ameliorate this, we remove usage of parse for variable completion that I introduced because it was causing EOF errors. use token parser instead, like the rest of the getAutocompleteSuggestions library. then we are able to parse incomplete graphql operations and SDL
  • only show variables when they are compatible with the argument
  • show variable types (previously it was just name)
  • show available variables before you start typing, including alongside enum or boolean completion options

cc: @yoshiakis

notes:

  • yes, we are adding type to CompletionItem results, but for the language server side, it will be converted to a string or set to undefined by the time the server is issuing the response to the LSP client (since it transmits over IPC, streams or sockets)

@acao acao force-pushed the feat/codemirror-interfaces branch from 1ba4541 to cbaefa5 Compare December 28, 2020 18:31
@codecov
Copy link

codecov bot commented Dec 28, 2020

Codecov Report

Merging #1747 (cd4ee22) into main (ed6c94c) will decrease coverage by 9.13%.
The diff coverage is 47.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1747      +/-   ##
==========================================
- Coverage   66.97%   57.83%   -9.14%     
==========================================
  Files          87       30      -57     
  Lines        4906     1537    -3369     
  Branches     1351      288    -1063     
==========================================
- Hits         3286      889    -2397     
+ Misses       1385      566     -819     
+ Partials      235       82     -153     
Impacted Files Coverage Δ
...kages/graphql-language-service-parser/src/types.ts 100.00% <ø> (ø)
...ervice-interface/src/getAutocompleteSuggestions.ts 58.65% <45.00%> (-15.46%) ⬇️
packages/codemirror-graphql/src/hint.js 94.73% <100.00%> (-0.22%) ⬇️
...language-service-utils/src/getASTNodeAtPosition.ts 18.75% <0.00%> (-81.25%) ⬇️
...guage-service-interface/src/getHoverInformation.ts 3.06% <0.00%> (-77.65%) ⬇️
...ql-language-service-interface/src/getDefinition.ts 11.90% <0.00%> (-74.10%) ⬇️
...aphql-language-service-interface/src/getOutline.ts 11.11% <0.00%> (-62.48%) ⬇️
...guage-service-utils/src/validateWithCustomRules.ts 60.00% <0.00%> (-40.00%) ⬇️
...ckages/graphql-language-service-utils/src/Range.ts 35.71% <0.00%> (-38.58%) ⬇️
...ge-service-interface/src/GraphQLLanguageService.ts 9.56% <0.00%> (-38.41%) ⬇️
... and 64 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 ed6c94c...cd4ee22. Read the comment docs.

- merge hint logic for codemirror back into `getAutocompleteSuggestions`
- this pulls in support for several upstream features in `getAutocompleteSuggestions`
- namely, the parsing and autocomplete logic for & syntax is working now
- don't show interface we're extending in completion
- rewrite variable completion to use streaming parser (no more EOF errors, sync parser doesn't work well here)
- ensure variable completion contains types
- add fragments option to codemirror-graphql
@acao acao force-pushed the feat/codemirror-interfaces branch from 2898378 to 0b8a54e Compare December 30, 2020 19:50
@acao
Copy link
Member Author

acao commented Dec 30, 2020

@yoshiakis another huge series of improvements in the last commit!

the variables completion was using graphql parse which just broke the heck out of codemirror-graphql. despite being new functionality, the EOF parse bug kept happening for incomplete queries, so i replaced it with runOnlineParser

several other edge cases fixed, all summarized in the commit message

last step is to test it all thoroughly with the LSP server! seems to be working working great in codemirror-graphql, graphiql and monaco-graphql with manual testing

@acao acao changed the title feat: merge completion logic for implements & feat: merge completion logic (for implements &, variables) Dec 31, 2020
@acao acao force-pushed the feat/codemirror-interfaces branch from 0b8a54e to 5717a7a Compare December 31, 2020 03:33
@acao acao force-pushed the feat/codemirror-interfaces branch from d243248 to 4b5c520 Compare January 2, 2021 23:47
@acao acao merged commit 0ac0a85 into main Jan 3, 2021
@acao acao deleted the feat/codemirror-interfaces branch January 3, 2021 15:47
@acao acao changed the title feat: merge completion logic (for implements &, variables) feat: completion logic (for implements & variables) Jan 26, 2021
@github-actions github-actions bot mentioned this pull request Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant