0no-co / gql.tada

🪄 Magical GraphQL query engine for TypeScript
https://gql-tada.0no.co
MIT License
2.52k stars 41 forks source link

docs: add vite-specific oddity #342

Open DerTimonius opened 1 month ago

DerTimonius commented 1 month ago

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

changeset-bot[bot] commented 1 month ago

⚠️ No Changeset found

Latest commit: 391f54dde9647cfa4ac54b9305bbfaff2e013a12

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 commented 1 month ago

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 commented 1 month ago

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

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 commented 1 month ago

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 commented 1 month ago

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?