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: add astro support for VSCode extension #3475

Merged
merged 4 commits into from
Jan 24, 2024
Merged

Conversation

XiNiHa
Copy link
Contributor

@XiNiHa XiNiHa commented Dec 16, 2023

Adds Astro syntax grammar support, language server parsing support, and misc wirings.

Copy link

changeset-bot bot commented Dec 16, 2023

🦋 Changeset detected

Latest commit: deccc6a

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 Minor
vscode-graphql Minor
vscode-graphql-syntax Minor
graphql-language-service-cli 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

Copy link

linux-foundation-easycla bot commented Dec 16, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@XiNiHa XiNiHa force-pushed the feat/astro branch 2 times, most recently from 8d0ed2d to 0b16e7a Compare December 16, 2023 16:53
Copy link
Member

@acao acao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome work! you even covered the esbuild script, very comprehensive, thank you!

@acao
Copy link
Member

acao commented Jan 7, 2024

@XiNiHa just a few issues with dependencies, spelling and linting to clear up it looks like

@acao
Copy link
Member

acao commented Jan 7, 2024

I'm about to merge a refactor for all of this that you will appreciate i'm sure, though it will cause a merge conflict

@acao
Copy link
Member

acao commented Jan 7, 2024

@XiNiHa enjoy! let me know if you want me to redo it

@XiNiHa
Copy link
Contributor Author

XiNiHa commented Jan 10, 2024

Sorry for being late to fix things up ;( I'll try to look at stuffs around this weekend

@acao
Copy link
Member

acao commented Jan 13, 2024

@XiNiHa no worries, take your time!

also I improved the grammars a bit. let me know if you want to pair on this or have any questions.

@XiNiHa XiNiHa force-pushed the feat/astro branch 2 times, most recently from 5a798e0 to c776e7e Compare January 20, 2024 15:05
@XiNiHa
Copy link
Contributor Author

XiNiHa commented Jan 20, 2024

The language server doesn't work due to ardatan/graphql-tools#5781 not being merged yet.

@acao
Copy link
Member

acao commented Jan 21, 2024

@XiNiHa we have our own code parsing currently, but graphql-tools are also parsing files on their own, we don't use that result

@acao
Copy link
Member

acao commented Jan 21, 2024

two things!

  1. can you add astro tests in findGraphQLParser-test.js
  2. I'm not sure why the graphql-language-service-server tests are failing for a missing astrojs-compiler-sync, even though it is added to package.json and seems to be reflected in yarn. can you try running yarn install again to update the lockfile?

@XiNiHa
Copy link
Contributor Author

XiNiHa commented Jan 22, 2024

I'm not sure why the graphql-language-service-server tests are failing for a missing astrojs-compiler-sync

Looks like a typo in the package.json of astrojs-compiler-sync, will make a PR on there

@acao
Copy link
Member

acao commented Jan 23, 2024

awesome work!

sidenote: i think the issue with svelte2tsx is twofold - the language server needs to specify typescript as a dependency so that it works in non vscode contexts, and with vscode, we need to upgrade vsce as you have done and mark typescript as external. if we try to make it internal to the esbuild bundle it breaks as well. i can show you how to debug definitively - the vscode extension runner does not reflect the esbuild bundled state

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (88ae243) 55.94% compared to head (deccc6a) 55.74%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3475      +/-   ##
==========================================
- Coverage   55.94%   55.74%   -0.21%     
==========================================
  Files         113      114       +1     
  Lines        5291     5312      +21     
  Branches     1441     1444       +3     
==========================================
+ Hits         2960     2961       +1     
- Misses       1903     1923      +20     
  Partials      428      428              
Files Coverage Δ
...s/graphql-language-service-server/src/constants.ts 100.00% <ø> (ø)
...hql-language-service-server/src/findGraphQLTags.ts 82.71% <ø> (ø)
...aphql-language-service-server/src/parsers/astro.ts 4.76% <4.76%> (ø)

@acao
Copy link
Member

acao commented Jan 24, 2024

@XiNiHa this looks great! I will go ahead and merge, and then test the .vsix bundle manually just to be 100% certain. I think we can merge this and fix the svelte parsing in a single release

@acao
Copy link
Member

acao commented Jan 24, 2024

I will take care of adding the tests in a follow up PR, but I might ping you later to make sure I've covered all the cases

@acao acao merged commit 98af530 into graphql:main Jan 24, 2024
14 checks passed
@acao acao mentioned this pull request Jan 24, 2024
@XiNiHa XiNiHa deleted the feat/astro branch January 25, 2024 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants