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

docs: add vite-specific oddity #342

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DerTimonius
Copy link

Summary

First of all, thanks for this awesome tool!

This PR will add a small section to the installation docs: while installing this in a Vite project, I realized after some trial and error that it is also necessary to add the plugin to the tsconfig.app.json file, as you won't get type hints otherwise (yes, I have spent way too long to figure that out).

Set of changes

Just small changes to the docs

Copy link

changeset-bot bot commented Jul 10, 2024

⚠️ No Changeset found

Latest commit: 391f54d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kitten
Copy link
Member

kitten commented Jul 10, 2024

Can you clarify your setup please?

This really seems like an oddity of your own project. For instance, take our examples in this repo. They're pretty standard Vite projects and they don't contain a tsconfig.[name].json at all, but only a single tsconfig.json: https://github.com/0no-co/gql.tada/tree/main/examples/example-pokemon-api

Furthermore, by default renaming the tsconfig.json means that they won't be picked up by your editor, so out of the box, this wouldn't actually work unless your main tsconfig.json refers to the other tsconfig.app.json, e.g. with an extends.

I'm more inclined to ask maybe what gql.tada doctor said for you in this case. If the output here doesn't warn you about this then we can probably work on that, but as it is, the docs addition doesn't quite make sense to me. Sorry! ❤️

@DerTimonius
Copy link
Author

DerTimonius commented Jul 10, 2024

I actually did nothing more than run pnpm create vite and it created three different tsconfig files:

  • tsconfig.json
  • tsconfig.node.json
  • tsconfig.app.json

when I ran gql.tada doctor it didn't give me any errors, and the creation of the correct types of the query worked, but there were no type hints.

only after I put the plugin section into the tsconfig.app.json file, the type hints were actually there.

edit:
this is the tsconfig file it initially created:

{
  "files": [],
  "references": [
    {
      "path": "./tsconfig.app.json"
    },
    {
      "path": "./tsconfig.node.json"
    }
  ]
}

@kitten
Copy link
Member

kitten commented Jul 10, 2024

when I ran gql.tada doctor it didn't give me any errors

It likely won't if you use both references and then put compilerOptions in the same tsconfig.json.

The problem here is, create-vite might create a structure like this, but it might not. This really isn't specific to Vite and I wasn't aware one of their templates would do this. However, that does mean we can't give a general instruction for Vite that says this.

On the other hand, I don't think we can point out and explain references effectively in our docs. There's a lot of things in TypeScript to explain.
I can see that this is done quote often (however, not very consistently) in their templates only to have a specific config for the vite.config.ts format/file, which seems a little heavy handed.

I suppose we can add a check for references to gql.tada doctor, however, that also won't always be perfect since there's a lot of separate valid cases with references that we can't warn about once the configuration is accidentally added to a root tsconfig.json. I suppose we'll have to start checking maybe whether compilerOptions is otherwise empty with references being present 🤔

But I'd prefer to maybe find a solution with gql.tada doctor first rather than a docs update, since it feels like the docs update would have to otherwise be really oddly phrased, along the lines of "if you have an empty tsconfig.json that only contains references then go to the one that targets all your project files first [etc etc]".
Basically, I don't think a docs update is suitable since we're talking about a config entry is very deliberate and only causes trouble if it's set up for you without you having knowledge about what references does. (Part of why I think it's a little weird they set this up by default in these templates)

Let me know how that sounds ✌️ Maybe you can confirm that you initially added the compilerOptions.plugins section to a file looking like this first? https://github.com/vitejs/vite/blob/main/packages/create-vite/template-vue-ts/tsconfig.json
Then we can probably make a plan for what gql.tada doctor would have to check

@DerTimonius
Copy link
Author

Hm..I did not think of the differences in the created tsconfigs by Vite (insert relevant xkcd about standards here).

Yes, the initial tsconfig.json looked just like the file you linked.

I agree that checking for referenced tsconfig files makes sense. The problem is not only gql.tada doctor, but also other gql.tada CLI tools like check or generate output when I would remove the compilerOptions field from the tsconfig.json and only keep it in tsconfig.app.json.

So in my opinion, there are two different issues with Vite as a user:

  1. when following the instructions in the docs (or running gql.tada init), the package works without type hints
  2. if there are multiple tsconfig files, you might end up putting the compilerOptions into the wrong file and the CLI won't work

Therefore there are two different solutions, with the first one being easier:

  1. add more generic instructions to the docs on what to do if you have multiple tsconfig.json files (in this case, just add the compilerOptions twice)
  2. add checks to the CLI, if there are references to other tsconfig files

What do you think? What approach do you prefer?

@martywallace
Copy link

Just spent a couple hours pulling my hair out over intellisense not working in the graphql(...) block and randomly stumbled upon this MR which solved my issue (moved the plugin from tsconfig.json to tsconfig.app.json), so even if it doesn't get merged, thank you!

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.

3 participants