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

[feature request] svelte files (Houdini + Svelte-Kit) #168

Closed
Hebilicious opened this issue Jan 13, 2024 · 8 comments
Closed

[feature request] svelte files (Houdini + Svelte-Kit) #168

Hebilicious opened this issue Jan 13, 2024 · 8 comments

Comments

@Hebilicious
Copy link

Hebilicious commented Jan 13, 2024

Hello there!
Thank you for working on such an important part of the graphQL tooling ecosystem.

I'm currently working on a SvelteKit project that uses Houdini as it's graphQL client.
GraphQLSP almost work there, but not quite. A picture is worth a thousands words :

image

It appears that the *.svelte files are not detected (I don't get syntax highlighting), and therefore the fragments return with the TSError : Unknown fragment.

In Typescript files, it appears to work fine and provide a very neat and fast syntax highlighting.
Here's my config :

      {
        "name": "@0no-co/graphqlsp",
        "schema": "./graphql/schema.graphql",
        "templateIsCallExpression": true,
        "template": "graphql"
      }

What would be required to support the script tag of svelte files? (I assume this might extend to the script tag of vue SFC too)

I'm happy to try and contribute the feature, if you can point me in the right direction.

Edit: Investigate it a little more, there's this svelte typescript plugin here, https://github.com/sveltejs/language-tools/blob/master/packages/typescript-plugin/README.md , but I'm not sure how they can "work together".

@Hebilicious Hebilicious changed the title [feature request] Houdini Support (Svelte-Kit) [feature request] svelte files (Houdini + Svelte-Kit) Jan 13, 2024
@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 14, 2024

Hey,

Currently we have no support for reading .vue or .svelte files, that being said I expected functionality like the plugin you mentioned to decorate i.e. getSemanticDiagnostics and pipe in a TypeScript AST so other plugins could properly leverage a TS-AST for .svelte. Upon closer inspection of the svelte plugin it looks like rather than that they inspect the .svelte file and in case they can parse it return their own diagnostics where we for instance merge them with the original.

I don't know the specifics though so maybe it's not possible to tell TypeScript to use a different type of AST, ... I am not deeply familiar with folks working in the LSP ecosystem for these languages, I can try asking around as I would love to be able to support anyone using GraphQL.

I saw the following pop up quite often 😅

        // Diagnostics inside Svelte files are done
        // by the svelte-language-server / Svelte for VS Code extension
        if (isSvelteFilePath(fileName)) {
            return [];
        }

I think in theory what would need to happen is we'd need to convert the .svelte file into a valid TypeScript AST and pipe that into the plugin, I however doubt that the conversion or cross-file interactions will be easy to make functional. I'm not sure whether there is a way to compose LSP plugins but this feels like something I personally fear where custom syntaxes are created, a lot of the ecosystem might end up being re-created and the collaboration with existing syntaxes might fall by the wayside.

Maybe there is a way where when we see a .svelte/.vue import to offload logic to the svelte or vue compiler so we know what is exported/imported from there but we'd still end up re-creating a lot of the LSP logic (not that I mind) but trying to be clear that this might end up as a pretty large feature where ts --> unknown syntax would analyse the ts file and follow the import into the unknown syntax where after the work is offloaded to the compiler in question for said syntax. When we are on unknown syntax I assume that the LSP won't trigger (?) if it does we'd have to just offload the work and if it doesn't we'd have to rebuild the logic in the svelte-language-server / Svelte for VS Code extension as said in the above comment.

Sorry for the musings, I personally haven't used Svelte/Vue too much so trying to give you everything that pops to mind about this issue and maybe that sparks an idea on your side.

CC @AlecAivazis in case you got ideas here

@Hebilicious
Copy link
Author

Hebilicious commented Jan 14, 2024

@JoviDeCroock

Thanks for the detailed answer.
My knowledge of LSP and typescript plugins is very limited, and I have no idea if we can "pipe" or "compose" LSP together. If that's possible, then this would probably make things easy...

I will ask around the vue and svelte folks that I know that are working on language server like Volar, which powers vue and astro VScode extensions ... https://volarjs.dev/core-concepts/embedded-languages/ , they might have some ideas on how this can be achieved.

My gut feeling tell me that a lot of stuff like that has been done by the vue team, if you take a look at this https://github.com/vuejs/language-tools/blob/master/packages/typescript-plugin/src/index.ts it doesn't seem that complicated with Volar... So maybe adding support directly into GraphQLSP is possible with such an approach?
I want to point out that I could be totally wrong as I don't really get how these things work yet 😅

I will gather some information during the week and will come back if I hear something helpful. I've also pinged the Houdini folks on their discord as they have some insight... But still a more "generic" solution that works with embedded typescript blocks would be fantastic.

In the meantime, is there a way to silence the unknown fragment error or to make it a warning ? The developer experience provided in the TS file is so amazing, would love to not have to turn the plugin off.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 14, 2024

@Hebilicious there is a setting shouldCheckForColocatedFragments which you can put to false, not sure about other approaches. These are already warnings btw https://github.com/0no-co/GraphQLSP/blob/main/packages/graphqlsp/src/diagnostics.ts#L344

@Hebilicious
Copy link
Author

@Hebilicious there is a setting shouldCheckForColocatedFragments which you can put to false, not sure about other approaches. These are already warnings btw https://github.com/0no-co/GraphQLSP/blob/main/packages/graphqlsp/src/diagnostics.ts#L344

Using v0.15, I'm still seeing this as an error
image

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 14, 2024

Ah yes; that's validation failing... not much we can do there apart from the client preset or something else that defines fragments centrally. Not sure how houdini works but with the client preset we locate the fragments centrally and as it's not in the selection we can't discover it. You might want to add support for houdini's special fragment thing.

So to describe the high level way of working of two approaches in the LSP

  • Client preset all fragments and queries get written to a central file which we can use to resolve FragmentSpreads in documents
  • Gql.tada we can resolve fragments recursively from the second argument
  • Tagged templates we can resolve recursively from template interpolations

I have no idea where with houdini we have to find the FragmentDefinition and it doesn't look like there is a reference in scope to lead us to these definitions 😅 if someone from the houdini crew could chime in here then we could have a way forward as the code to alter is quite convenient to reason about, it's here for the two first approaches and the PR to add that is here.

From what I can tell here we should be able to find the FragmentDefinition in ./$houdini as that should have the global type that overrides graphql() to have a mapping of document to type.

@AlecAivazis
Copy link

Sorry for chiming in a bit late here, it's been a crazy week. Houdini walks down the user's filesystem to find all of fragment definitions. We can definitely generate a centralized mapping somewhere if that helps the LSP find them. It's not immediately clear to me what that structure would look like from the examples but i haven't had my coffee yet. That being said, I'm more than happy to help clarify anything or answer any questions that pop up along the way.

@xamir82
Copy link

xamir82 commented Feb 29, 2024

Does this really even have anything to do with Houdini?
Doesn't it just some down to Svelte language tools not supporting TypeScript plugins (a.k.a sveltejs/language-tools#905)?

@JoviDeCroock
Copy link
Member

Svelte is from now on supported in our CLI https://github.com/0no-co/gql.tada, I don't think as we are moving more and more towards tada usage that we'll add explicit support for Houdini. In the future we might be able to add support for Svelte/Astro/Vue through volar but I will hold off from that for now and track this issue in #242 as Astro is already using volar.

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

No branches or pull requests

4 participants