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

Allow generic defaults for TSESLint.RuleModule? #4134

Closed
3 tasks done
JoshuaKGoldberg opened this issue Nov 12, 2021 · 1 comment · Fixed by #4135
Closed
3 tasks done

Allow generic defaults for TSESLint.RuleModule? #4134

JoshuaKGoldberg opened this issue Nov 12, 2021 · 1 comment · Fixed by #4135
Labels
package: utils Issues related to the @typescript-eslint/utils package triage Waiting for maintainers to take a look

Comments

@JoshuaKGoldberg
Copy link
Member

  • I have tried restarting my IDE and the issue persists. N/A
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

Going off the code snippet here: #4124 (comment)

This gives an error [playground link]:

import { TSESLint } from '@typescript-eslint/experimental-utils';

// Generic type 'RuleModule<TMessageIds, TOptions, TRuleListener>' requires between 2 and 3 type arguments.
export const rule: TSESLint.RuleModule = {

Although I personally would use RuleCreator, or failing that specify those TMessageIds and TOptions type arguments, it is inconvenient for folks getting started with writing rules to have to specify them. It would make for smoother documentation if we didn't have to walk people through setting them up until they're needed.

Proposal:

interface RuleModule<
-  TMessageIds extends string,
+  TMessageIds extends string = string,
-  TOptions extends readonly unknown[],
+  TOptions extends readonly unknown[] = [],

Versions

package version
@typescript-eslint/experimental-utils 5.3.1
TypeScript N/A
node N/A
@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for maintainers to take a look package: utils Issues related to the @typescript-eslint/utils package labels Nov 12, 2021
@bradzacher
Copy link
Member

The problem with specifying defaults for TMessageIds is that then by default there is no typesafety over them.

const rule: TSESLint.RuleModule = {
  meta: {
    messages: {
      foo: 'FOOOO',
    },
  },
  create(context) {
    // no TS error :( :( :( 
    context.report({ messageId: 'obviouslyMissingId' });
  },
};

ruleTester.run('my-rule', rule, {
  valid: [],
  invalid: [
    {
      code: 'const x = 1;',
      // no TS error :( :( :( 
      errors: [{ messageId: 'obviouslyMissingId' }],
    },
  ],
});

Which is one of the major reasons we went with the rule creator function - because TS can infer the type for you!

const rule = createRule({
  meta: {
    messages: {
      foo: 'FOOOO',
      bar: 'BAAAR',
    },
  },
  create(context) {
    // TS error because it inferred TMessageIds as 'foo' | 'bar'
    context.report({ messageId: 'obviouslyMissingId' });
  },
});

ruleTester.run('my-rule', rule, {
  valid: [],
  invalid: [
    {
      code: 'const x = 1;',
      // TS error because it inferred TMessageIds as 'foo' | 'bar'
      errors: [{ messageId: 'obviouslyMissingId' }],
    },
  ],
});

Options, OTOH, is okay to specify a default for. If you forget to specify options then you just end up with the options [] and you can't do anything with the rule!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: utils Issues related to the @typescript-eslint/utils package triage Waiting for maintainers to take a look
Projects
None yet
2 participants