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

gql.tada check reports an "Unkown fragment" error when fragments are imported in a TS file from a Vue SFC #210

Closed
3 tasks done
raphael-yapla opened this issue Apr 15, 2024 · 9 comments · Fixed by #232
Closed
3 tasks done

Comments

@raphael-yapla
Copy link

Describe the bug

We're trying to setup gql.tada to do a check in a CI job using the newly added check command (awesome addition btw!) but we're faced with a weird issue. The command will report incorrectly an error when fragments are imported from a different file type. This is what I've tested:

  • ✅ Fragment imported in a TS file from another TS file
  • ✅ Fragment imported in a Vue component from another Vue component file
  • ❌ Fragment imported in a TS file from a Vue component file

The IDE however, is fine with any kind of combination. I'm unsure where the problem lies exactly but it seems strange that the IDE would not report the error if both the CLI and IDE are using the LSP?

I've set up a reproduction that hopefully will help you track the source down.

Thank you so much for the awesome project, it might be the best package I've used in a long time!

Reproduction

https://github.com/raphael-yapla/gql-tada-vue-issue

gql.tada version

gql.tada v1.5.0

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@kitten
Copy link
Member

kitten commented Apr 15, 2024

Does that actually work in a regular tsc run or not?
I'm not super familiar with how Volar works, but for our check command that definitely won't just work out of the box.

See:

@raphael-yapla
Copy link
Author

No you'd have to use vue-tsc so that would makes sense.

The only thing I don't understand is how comes the check command works when importing to and from a Vue SFC file then?

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Apr 16, 2024

Yes, both for Svelte and Vue we could use their respective packages to do a prepass over all the files and make them into TS

  • SvelteTSX
  • VueTSC - here we'd probably need to dig deeper and discover how they use the vue-compiler to do this

Then add the TS equivalent of these files into ts-morph and run check - that being said for now these are open RFC's. I do think for the CLI we'll need to tackle this a bit differently than for the LSP. The LSP essentially will become a VSCode plugin/TSLang plugin built on volar if we want to support all these SFC formats.

From what I can tell we can even patch how TS gets its script snapshots, maybe that is a better solution for this as it would solve both the CLI and TS Plugin https://github.com/sveltejs/language-tools/blob/master/packages/typescript-plugin/src/index.ts. The volar way looks a lot simpler though https://github.com/vuejs/language-tools/blob/master/packages/typescript-plugin/index.ts

@kitten
Copy link
Member

kitten commented Apr 16, 2024

@JoviDeCroock Just to note this down somewhere, TS Morph seems to have an open issue for this but hasn't resolved this yet, but someone there basically pointed out the API to use for this on the TS side

@kitten
Copy link
Member

kitten commented Apr 18, 2024

Another comment here on this topic:
#231 (comment)

The gist of it is though that currently we have no timeline or implementation path yet for Vue, Astro, and Svelte support, and haven't deprioritised this yet

@JoviDeCroock
Copy link
Member

I have an initial working version for this in #232 - would love for this to be tested on larger projects though 😅

@KammererTob
Copy link

KammererTob commented Apr 19, 2024

I have tested the PR on my project, which contains >70 fragments and queries in various files (mostly .ts and .vue file), with nested fragments etc. The cache is now correctly populated, but it seems to break some typings (as far as i can tell only for fragments/queries which contain a fragment of another component). Here is an example (left without cache, right with cache).

image

"\n fragment EventDialog on Event {\n id\n name\n platform\n discipline\n ...EntityEditDialog\n }\n ": TadaDocumentNode<{ [x: string]: any; }, {}, { fragment: "EventDialog"; on: "Event"; masked: true; }>;

If you need more information please let me know. I can also try to reproduce this in the reproduction i've provided in the linked issue if helpful.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Apr 19, 2024

Was able to fix check and generate persisted but turbo is still a pain, looks like the type-checker isn't really willing to collaborate here 😅

EDIT: all fixed, we'll prepare this as an experimental feature

@raphael-yapla
Copy link
Author

Incredible, thank you so much for such a quick turnaround!

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 a pull request may close this issue.

4 participants