search

cometkim / gatsby-plugin-typegen

Let's give developers using GatsbyJS better DX with extreme type-safety
https://www.gatsbyjs.org/packages/gatsby-plugin-typegen/
MIT License
204 stars 25 forks source link

All files must be modules when the '--isolatedModules' flag is provided error in gatsby-types.ts #133

Closed Lisenish closed 3 years ago

Lisenish commented 3 years ago

When I run tsc --noEmit for type checks since I have isolatedModules config setting enabled I get this error All files must be modules when the '--isolatedModules' flag is provided error in gatsby-types.ts since generated file is just global declaration.

I can send PR with adding //@ts-ignore near /* eslint-disable */at the top of the generated file. I'm not sure if it's a good solution to this problem but I don't see the other way:

  1. Disable isolatedModules is not an option it's a good safety net
  2. Make gatsby-types.ts a module (by placing export before the namespace declaration) is a breaking change and ruins developer experience.
  3. Add ignore comment in gatsby-types.ts. The most simple solution, though I assume it will make harder to find potential problems in generated file during plugin development
  4. Probably the best solution: instead of generating gatsby-types.ts generate gatsby-types.d.ts, so that error doesn't apply anymore. And ask plugin users to add import './__generated__/gatsby-types' into their ./global.d.ts file. It's also breaking change and adds some friction during plugin setup (unless we don't automate it somehow). So I'm not sure if it's worth it but I think its the cleanest and right solution.
cometkim commented 3 years ago

Hey @Lisenish, thanks for reporting.

You already can use .d.ts instead of .ts by just changing the outputPath option.

{
  options: {
    outputPath: `src/__generated__/gatsby-types.d.ts`,
  },
}

Make gatsby-types.ts a module (by placing export before the namespace declaration) is a breaking change and ruins developer experience.

Yeah, this was the main concern of v2 (See https://github.com/cometkim/gatsby-plugin-typegen/pull/47) I looked for simplicity rather than designing complex options in 2.0. But it's easy to get mixed up, so I think there's still a chance for improvement :)

cometkim commented 3 years ago

Do you think it would be better to make the .d.ts output default?

Lisenish commented 3 years ago

@cometkim Oh, I somehow missed that option, thank you! :)

Yeah, I believe it should be the default actually since it's actually declaration what it is not a regular ts file. Also, I personally prefer to import it explicitly in global.d.ts so it's less magically accessible everywhere. The only thing I mentioned before the global.d.ts import should be clearly described in installation steps or ideally(?) inserted automatically.

I'm closing the issues since it's solved.

UPD I've found that import of these generated types in global.d.ts for some reason disabled other type declarations in global.d.ts. So the correct way to do this is to use /// <reference types="./__generated__/gatsby-types" /> instead of import