graphql-code-generator: generated type with generic which is never used

Hello 👋

noUnusedLocals and noUnusedParameters are still linting rules. The difference is that it’s enforced by TypeScript rather than the commonly used ESLint’s counter part, no-unused-var. This means it is still valid TypeScript.

Therefore, I recommend not linting the generated types. Here are some options to get around this issue:

Case 1: If you are using both TypeScript and ESLint:

• Turn off noUnusedLocals and noUnusedParameters
• Use ESLint’s no-unused-var
• Use .eslintignore to ignore generated files.

Case 2: If you are using TypeScript but not ESLint:

• Use add plugin to add // @ts-nocheck at the top of your file

Hello 👋

noUnusedLocals and noUnusedParameters are still linting rules. The difference is that it’s enforced by TypeScript rather than the commonly used ESLint’s counter part, no-unused-var. This means it is still valid TypeScript.

Therefore, I recommend not linting the generated types. Here are some options to get around this issue:

Case 1: If you are using both TypeScript and ESLint:

• Turn off noUnusedLocals and noUnusedParameters
• Use ESLint’s no-unused-var
• Use .eslintignore to ignore generated files.

Case 2: If you are using TypeScript but not ESLint:

• Use add plugin to add // @ts-nocheck at the top of your file

Hi @eddeee888, thanks for proposing a solution. Sincerely appreciated.

I’m about to implement it. As I’m in there doing it, a thought has come to mind. Wondering what you thought about it.

I understand your solution to Case 1 technically would solve the issue, but do you think it seems like the solution only ‘sort-of’ fixes the issue?

i.e. in Case 1, it appears the solution you propose leaves the issue in existence and requires consumers of graphql-code-gen in this scenario to re-configure the way they do linting of unused variables.

I understand what you’re saying when you explain that Typescript’s approach to linting with noUnusedLocals and noUnusedParameters is functionally equivalent to using ESLint’s approach to linting with no-unused-var, so that makes sense.

What I’m wondering though, wouldn’t it be better for the graphql-code-gen project to instead just not cause the linting issue in the first place? i.e. rather than requiring consumers of graphql-code-gen to have to deal with the issue by re-configuring their own linting configurations, why not just fix the issue at the source and not cause the issue by fixing the fact that RefType is declared but never used?

My concern is more about the inconvenience of it. I’m ok dealing with it, but that’s since I managed to find this GH issue report and read through it. What about everyone else that may experience this in the future? Are we just accepting that every single one of them will need to go through the same experience?

i.e. this is now what needs to go in my tsconfig.json file…

    // these next two false to deal with graphql-code-gen issue https://github.com/dotansimha/graphql-code-generator/issues/9498
    // let eslint handle these two instead using no-unused-vars via eslint:recommended
    "noUnusedLocals": false,
    "noUnusedParameters": false,

I’m ok to go ahead and make the fix on my end, but just thought I’d check to see what you thought of the idea?

Again, thank you, sincerely, for the effort in reading and understanding the issue and for the solution you have proposed. Your effort is very much appreciated. 🙏

noUnusedLocals and noUnusedParameters are still linting rules.

@eddeee888 you find me a TS core team member that’ll capitulate to your assertion that those configuration and compilation options are “linting” and I’ll concede that you’re in the right here. Otherwise, my take is that you’re closing and dismissing issues that are actual bugs in codegen. Other teams like https://github.com/drizzle-team/drizzle-orm are on the right side here, and updating their generators to avoid this same situation. Generating code which is not used is a bug, plain and simple.

I see! Unfortunately, it’s not an ESLint rule for me as I am using the following in the tsconfig.json:

    "noUnusedLocals": true,
    "noUnusedParameters": true,

As such, no choice but to add

// @ts-ignore

Directly above the line, hence the use of sed!

Thanks for your example dude!

@shellscape here’s my workaround:

async function tsIgnoreUnusedLocals() {
  const fileName = argv["file"];
  const fileContent = fs.readFileSync(fileName, "utf8");
  const lines = fileContent.split(/\r?\n/);
  const program = ts.createProgram([fileName], {
    noEmit: true,
    noUnusedLocals: true,
    noUnusedParameters: true,
  });
  const sourceFile = program.getSourceFile(fileName);
  if (!sourceFile) throw new Error(`File ${fileName} not found`);
  let offsetsToAddComment: Array<number> = [];
  const diagnostics = ts.getPreEmitDiagnostics(program, sourceFile);
  diagnostics.forEach((diagnostic) => {
    if (diagnostic.file && diagnostic.start != null) {
      const { line } = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
      if (diagnostic.code === 6133) {
        console.log(`🚀 ~ file: prune unused - adding ts-ignore to ${fileName}:${line}`);
        offsetsToAddComment.push(line);
      }
    }
  });

  // Remove duplicates and sort in descending order to avoid messing up line numbers while inserting
  offsetsToAddComment = Array.from(new Set(offsetsToAddComment)).sort((a, b) => b - a);
  offsetsToAddComment.forEach((lineNumber) => {
    lines.splice(lineNumber, 0, "// @ts-ignore");
  });
  const modifiedContent = lines.join("\n");
  fs.writeFileSync(fileName, modifiedContent);
}

@shellscape is right here. This isn’t just a tslint issue, this is a core typescript issue.

The current workaround requires an unnecessary degradation in TS’ safety features.

Understood @eddeee888 , especially about the complexities / opinions associated with linting.

Thanks for your thorough response @eddeee888, I can definitely understand the rationale around trying to keep things lean.

If this is the stance the project is taking, in that case, does it not make sense to bake in the rules that need disabling to the tool itself instead of requiring that users add it themselves?

If we’re essentially saying that graphql-codegen will not support those rules, it makes sense to me that those rules would automatically be disabled in all generated files

Hi @miller-productions and everyone in this thread, 👋

Thanks for taking the time to explain your view of the issue. 🙂

First, this RefType generic is used in some cases. Here’s a unit test generating the template and here’s the schema for that test. Basically, RefType is used to refer back to either ResolversTypes and ResolversParentTypes. (There’s a section at the end with reasoning why the current option was chosen).

There are many levels of strictness when it comes to linting in TypeScript projects. If we have to generate code in a way that works for all consumer linting standards, we’d spend a lot more time fixing small linting tweaks, rather than making features.

So, my personal view has been to ignore these files from linting. If it’s valid TypeScript syntax, it’s not a bug.

My opinion about noUnusedLocals and noUnusedParameters:

Whether or not codegen is used, I’m against turning on these options and the ESLint rule together. The reason: multiple tools don’t need to be doing the same thing.

So, I don’t think it’s fair to say we have to turn these options off because of codegen. I think it’s a good practice in general. 🙂

export type ResolversUnionTypes = {
  ChildUnion: ( Child ) | ( MyOtherType );
};

export type ResolversUnionTypes<RefType extends Record<string, unknown>> = {
  ChildUnion: ( Child ) | ( MyOtherType );
  MyUnion: ( Omit<MyType, 'unionChild'> & { unionChild?: Maybe<RefType['ChildUnion']> } ) | ( MyOtherType );
};

 export type ResolversUnionTypes = {
  ChildUnion: ( Child ) | ( MyOtherType );
  MyUnion: ( Omit<MyType, 'unionChild'> & { unionChild?: Maybe<ResolversTypes['ChildUnion']> } ) | ( MyOtherType );
};

export type ResolversUnionParentTypes = {
  ChildUnion: ( Child ) | ( MyOtherType );
  MyUnion: ( Omit<MyType, 'unionChild'> & { unionChild?: Maybe<ResolversParentTypes['ChildUnion']> } ) | ( MyOtherType );
};

Same problem here.

'RefType' is declared but its value is never read.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
export type ResolversInterfaceTypes<RefType extends Record<string, unknown>> = ResolversObject<{
 ...
}>;

I’m using the same @ts-ignore workaround for now, but it’s not ideal.

I’m also seeing this. @jaw187 do you have any workarounds?

For anyone else wondering, you can just use this script:

    sed -i '/^export type ResolversUnionTypes.*/i \\/\\/ @ts-ignore' server/schema/typeDefs/index.ts