graphiql icon indicating copy to clipboard operation
graphiql copied to clipboard

fix(language-service): improve outline in embedded graphql

Open forivall opened this issue 11 months ago • 6 comments

In VSCode, I noticed that the editor sticky scroll headers were showing the wrong headers, and I tracked it down to the GraphQL language support. This also fixes the locations when you navigate to graphql sections using breadcrumbs or the outline panel.

I also emit outlines for all embedded graphql documents, rather than just the first one. This only works properly when the location data is correct (i tried this before fixing the location data, and it lead to some interesting results)

This PR also includes some additional typing and performance improvements, but I saw some mentions of an ongoing refactoring in the commit history, so I have just the targeted changes in a branch: https://github.com/graphql/graphiql/compare/main...forivall:graphiql:outline-embedded-offset-simple

I only added tests for my performance improvement, but let me know if you need more tests!

forivall avatar Jan 06 '25 04:01 forivall

🦋 Changeset detected

Latest commit: 92e47709d5257fb00ab4bc51907ba4a1ac5e499c

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

This PR includes changesets to release 4 packages
Name Type
graphql-language-service-server Patch
graphql-language-service Patch
vscode-graphql Patch
codemirror-graphql 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

changeset-bot[bot] avatar Jan 06 '25 04:01 changeset-bot[bot]

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: forivall / name: Emily Marigold Klassen (45c48d0e1d21cb8822a2938e92a03c02e6126f00, 92e47709d5257fb00ab4bc51907ba4a1ac5e499c, a320ad3324057413e8bbad3ae990cb4e99d987a5, ad0d6c511767d7e7684299c80101dc411e041d6c, b4f0bf1fbbd20f7a5ca85bb82c173ef778ec14b7, db47f700d94d7930597c5b505d4949c51d131d08, e114b6c3b4fd241c4126c9e247d6ab71620c14c6)

Codecov Report

Attention: Patch coverage is 86.36364% with 9 lines in your changes missing coverage. Please review.

Project coverage is 64.21%. Comparing base (614ff8b) to head (1f72129). Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ql-language-service-server/src/MessageProcessor.ts 81.81% 4 Missing :warning:
...ckages/graphql-language-service/src/utils/Range.ts 83.33% 3 Missing :warning:
...aphql-language-service/src/interface/getOutline.ts 90.47% 2 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3848      +/-   ##
==========================================
+ Coverage   63.98%   64.21%   +0.22%     
==========================================
  Files          35       35              
  Lines        3088     3124      +36     
  Branches      951      962      +11     
==========================================
+ Hits         1976     2006      +30     
- Misses       1107     1113       +6     
  Partials        5        5              
Files with missing lines Coverage Δ
...guage-service-server/src/GraphQLLanguageService.ts 87.84% <100.00%> (+0.06%) :arrow_up:
...aphql-language-service-server/src/parseDocument.ts 100.00% <ø> (ø)
...ql-language-service/src/interface/getDefinition.ts 92.59% <100.00%> (ø)
...aphql-language-service/src/interface/getOutline.ts 76.92% <90.47%> (+3.84%) :arrow_up:
...ckages/graphql-language-service/src/utils/Range.ts 85.71% <83.33%> (-2.17%) :arrow_down:
...ql-language-service-server/src/MessageProcessor.ts 88.72% <81.81%> (-0.26%) :arrow_down:

codecov[bot] avatar Jan 06 '25 05:01 codecov[bot]

awesome work! my outline impl was originally quite half baked, thank you for fixing it!

acao avatar Jan 11 '25 06:01 acao

rebasing this now - one spec file in the LSP server will fail, because of a known test bug, but I know this was already passing when you first submitted it, so it should work out fine!

acao avatar Mar 14 '25 02:03 acao

oof, yeah there were some upstream changes I think, let me take a closer look here within the week if you don't have time to

acao avatar Mar 14 '25 02:03 acao