feat: definePlugin

[JoshuaKGoldberg]

Summary

Adds a definePlugin function to @eslint/plugin-kit as a recommended way to group plugin configs, meta, and rules.

Related Issues

• Change Request: Provide a definePlugin function in @eslint/plugin-kit rewrite#176
• Enhancement: Support Lazy Loading Rules typescript-eslint/typescript-eslint#11029
• Support Lazy Loading Rules vuejs/eslint-plugin-vue#2732

[molisani]

When defining the rules objects for a plugin, it doesn't seem like the names are prefixed here. https://eslint.org/docs/latest/extend/plugins#configs-in-plugins

This would just be { "rule": { ... } } and then the example/ would be set when the config registered this plugin as "example": pluginObject.

It would be convenient for this utility to have some way of either automatically prefixing these rules when defining the config, or otherwise enforce that all of the rules exist.

[JoshuaKGoldberg]

Agreed, that would be nice. It would be pretty ergonomic IMO if the new rules could take in an array of rules objects.

The tricky point is, what would assign the rule name for the rules then? Rule meta objects don't have a name field in ESLint core. typescript-eslint's RuleCreator does but that's non-standard.

I would personally be in favor of:

• Adding an optional meta.name: string property
• defineConfig allowing an array of rules rather than object iff each has a unique meta.name

[molisani]

I don't think there's anything wrong with having to define the rules as a map of "non-namespaced name" to rule object. It's when we have to add the namespace to specify the rules in the configs that this gets potentially messy.

I like the approach of having meta.docs.recommended to determine the recommended config, but perhaps that could go all the way to its conclusion: each rule could locally define (meta) which configs it should be in?

[JoshuaKGoldberg]

each rule could locally define (meta) which configs it should be in?

Yeah! That's what I'm going for with meta.docs.recommended. The first version of this RFC just has true support for a recommended. From feedback in https://github.com/eslint/rfcs/pull/132/files#r2059307675 I'm expanding it now to also support valid severities such as "error" and "warn". I suspect going further than that would get into tricky territory. Some plugins define specific configs as supersets of others, and having to describe all of those in each individual rule's meta is a lot of duplicate & heavy work.

[michaelfaith]

I'm not sure I fully understand why it would be limited to just rules with error severity. I know some really popular plugins that have rules with severity warn in their recommended config (e.g. eslint-plugin-jest). Idea: If meta.docs.recommended was expanded to be boolean | 'warn' | 'error' (and 1 | 2?), then the logic could go something like

{
//...
  rules: Object.fromEntries(
    Object.entries(plugin.rules)
      .filter(([, rule]) => rule.meta.docs?.recommended)
      .map(([ruleName, rule]) => {
        const recommended = rule.meta.docs.recommended;
        if (["warn", "error"].includes(recommended)) {
          return [ruleName, recommended];
        } else {
          return [ruleName, "error"];
        }
    }),
  )
//...
}

[fasttime]

Mind that rules in a config (therecommended config) can be configured with a severity and options, neither of which is expressed in the rule's metadata.

[JoshuaKGoldberg]

In my mind, there are kind of two general paths plugins might want to go down:

• "Automatic": they don't want to be opinionated about specifics like error vs. warning. They just want it to be set up in a generally standard way.
• "Opinionated": they do care about things like error vs warning. They need tools to set up their opinions in a straightforward way, but are almost certainly going to need to customize things.

For "automatic" plugins, I think this is an opportunity to set one default behavior that works for the recommended starting plugin shape. Note that this is just the default; it's not a requirement. But it's a good way to enforce (what IMO is already) the default behavior of plugins that users generally expect.

For "opinionated" plugins, they don't have to go with these defaults - they can always configure with the stuff mentioned later on.

In other words: I think these these defaults need to make sense for the common case of plugins, but don't need to encapsulate every common customization (added rule options, non-error severity, etc.).

This stance is not very documented in the RFC right now...

neither of which is expressed in the rule's metadata.

But they could add it to the metadata, right? I think this is actually a not-rare practice. E.g. typescript-eslint does it for customizations to strict configs.

[michaelfaith]

For "automatic" plugins, I think this is an opportunity to set one default behavior that works for the recommended starting plugin shape. Note that this is just the default; it's not a requirement. But it's a good way to enforce (what IMO is already) the default behavior of plugins that users generally expect.

I definitely agree that having some default behavior that may not cover all use cases is valuable, but if a super common use case isn't covered, then its value is significantly diminished. Having rules with warn severity in a recommended config is super common, and so I fear that if that isn't covered in some way by the default behavior, then there'd be a lot less people able to use the default behavior.

[nzakas]

I agree with @michaelfaith here - I don't think autogenerating configs where the severity is "error" and cannot be overridden makes much sense. A concrete case: the @eslint/css plugin has a recommended config where only one rule is set to "warn". It's not a great experience if having one rule that needs to be set to "warn" means I need to fully define the entire config.

It does seem like if we want to go down the route of fully-automatic configs then meta needs a significant overhaul. To that end, it's worth thinking about whether or not a v1 of definePlugin() needs this capability.

[JoshuaKGoldberg]

My "never use warn" bias is showing 😄. In the latest commit I expanded the default generated config design to allow either true or any valid severity. So having meta.docs.recommended: 'warn' will natively work to set that as the severity in the recommended config. Is that a sufficiently scoped default behavior?

Linking to https://github.com/eslint/rfcs/pull/132/files#r2070777537: I suspect going further than that would get into tricky territory. Some plugins define specific configs as supersets of others, and having to describe all of those in each individual rule's meta is a lot of duplicate & heavy work.

[michaelfaith]

That seems like a good balance. 👍

In the latest commit...

Was that pushed? I'm not seeing that update

[JoshuaKGoldberg]

Blagh sorry, just pushed now.

[michaelfaith]

Looks good!

[michaelfaith]

This seems like an adjacent but separate conversation.

[nzakas]

Thanks for all the work and research you put into this.

At a high-level, I can see the value of a definePlugin() helper function that:

• provides type safety
• makes it easier to define configs
• enforces best practices (requiring meta.name, meta.version, and meta.namspace

I'm less convinced with some of the more ambitious parts of this proposal such as:

• lazy-loading rules
• autocreation of configs

By way of example, defineConfig()s goals were primary to provide type safety and add an extends key, so I'm wondering if maybe it makes sense to scale down definePlugin()'s MVP?

[nzakas]

I agree with @michaelfaith here - I don't think autogenerating configs where the severity is "error" and cannot be overridden makes much sense. A concrete case: the @eslint/css plugin has a recommended config where only one rule is set to "warn". It's not a great experience if having one rule that needs to be set to "warn" means I need to fully define the entire config.

It does seem like if we want to go down the route of fully-automatic configs then meta needs a significant overhaul. To that end, it's worth thinking about whether or not a v1 of definePlugin() needs this capability.

[nzakas]

A downside of this approach is that you can't tell just by looking at this code whether or not the plugin exports any configs.

[nzakas]

Can you provide some examples for these?

[JoshuaKGoldberg]

• A rule object ...: the typescript-eslint equivalent
• An array containing a rule object ...: the eslint-plugin-storybook equivalent
• An object with string keys and ...: the eslint-plugin-markdown equivalent

Are those what you're looking for?

[nzakas]

More looking for actual code examples.

[JoshuaKGoldberg]

I've come back to this comment a few times and have failed to parse each time. For "examples" / "actual code examples", the code blocks later on in the doc are what I would think to post. What info is being requested that's not being provided already?

[nzakas]

I'd like examples right in this spot. The wording here is confusing and I can't really map each bullet point to any specific example below.

• An array containing a rule object and a severity to specify that rule as

Are you saying an array of arrays? It's unclear from the context. Imagine instead you put:

• An array containing a rule object and a severity to specify that rule as, for example: ...

And the example was right there. That's what I'm looking for.

[JoshuaKGoldberg]

Thanks, makes sense!

[JoshuaKGoldberg]

Looking back at this, wow, yes, it was not clear. I overhauled the examples and explanations in f8e6acf.

[JoshuaKGoldberg]

I'm less convinced with some of the more ambitious parts of this proposal such as:

Added:

• autocreation of configs: 5d25577
• lazy-loading rules: feat: definePlugin #132 (comment) ✅