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

Change Request: Creating variants of a rule to apply to separate files #18242

Closed
1 task done
MJez29 opened this issue Mar 28, 2024 · 2 comments
Closed
1 task done

Change Request: Creating variants of a rule to apply to separate files #18242

MJez29 opened this issue Mar 28, 2024 · 2 comments
Labels
core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@MJez29
Copy link

MJez29 commented Mar 28, 2024

ESLint version

v8.57.0

What problem do you want to solve?

On a high level

I would like to be able to have multiple configurations for a rule run on different but potentially overlapping file sets

Use case 1: Composable no-restricted-imports

I have a monorepo with multiple types of applications (react apps, node servers, etc). I am using flat configs (and loving them!) to create configs for different topics (ex. typescript, react, next). Relevant configs are then composed in an app's eslint.config.mjs.

For certain rules such as no-restricted-imports I want multiple topic configs to restrict relevant imports

export const reactConfig = {
  files: "**/*.tsx?",
  rules: {
    "no-restricted-imports": ["error", {
       paths: [{
         name: "<dependency-that-we-are-deprecating>"
       }]
    }]
  }
}

export const nextConfig = {
  files: "**/*.tsx?",
  rules: {
    "no-restricted-imports": ["error", {
      paths: [{
        name: "react-router-dom"
      }]
    ]
  }
}

export const testConfig = {
  files: "**/*.test.tsx?",
  rules: {
    "no-restricted-imports": ["error", {
      paths: [{
        name: "<dependency-that-we-have-a-wrapper-for>"
      }]
    ]
  }
}

// eslint.config.mjs
export default [
  reactConfig,
  nextConfig,
  testConfig
]

The problem is that only the restrictions from the last config will apply. I want the restrictions from all to apply.

It is possible to manually process these in the eslint config

[
  {
    files: ["**/*.tsx?", "!**/*.test.tsx"],
    rules: {
      "no-restricted-imports": ["error", { paths: [
        ...reactConfig.rules["no-restricted-imports"][1], 
        ...nextConfig.rules["no-restricted-imports"][1] 
      }]
    }
  }, {
    files: ["**/*.test.tsx?"],
    rules: {
      "no-restricted-imports": ["error", { paths: [
        ...reactConfig.rules["no-restricted-imports"][1], 
        ...nextConfig.rules["no-restricted-imports"][1],
        ...testConfig.rules["no-restricted-imports"][1] 
      }]
    }
  }
]

But this requires users to know the internals of these configs and is very easy to get wrong or unintentionally break, and becomes very difficult to manage as the number of configs/fileset permutations increase

Use case 2: Warning + Errors

Extending the reactConfig example there are some dependencies that we flat out do not want to use and others that we are deprecating. We would like to flag the former as error but the latter with warn

What do you think is the correct solution?

The easiest solution here is to create separate rules for the different configs. This can be accomplished by duplicating the implementation of no-restricted-imports but this adds maintenance overhead of keeping the rules up-to-date with eslint's implementation.

An idea that I had and am trialing in my repo is to create a factory that creates custom no-restricted-imports rules by wrapping the actual implementation of no-restricted-imports

// eslint does not list this file in `exports` so we have to import it directly
import noRestrictedImports from "../../../../node_modules/eslint/lib/rules/no-restricted-imports.js";

/**
 * Creates a custom `no-restricted-imports` rule with the given options.
 *
 * `options` should be the 2nd parameter when configuring the default no-restricted-imports rule.
 * ```
 * "no-restricted-imports": ["error", options]
 * ```
 */
export function createNoRestrictedImportsRule(options) {
  return {
    meta: {
      ...noRestrictedImports.meta,
      schema: [],
    },
    create: (context) => {
      return noRestrictedImports.create({
        ...context,
        options: [options],
      });
    },
  };
}

const noRestrictedReactImportsRule = createNoRestrictedImportsRule({
     paths: [{
       name: "<dependency-that-we-are-deprecating>"
     }]
  })

const reactConfig = {
  plugins: {
    "@myorg-react": {
      rules: {
        "no-restricted-imports": noRestrictedReactImportsRule
      }
    }
  },
  rules: {
    "@myorg-react/no-restricted-imports": ["error"]
}

This is hacky because I have to absolutely import the file to bypass the exports in the package.json not exposing this file. If my use-case is valid I would like eslint to support creating variants of rules with specific options baked into them. This can be accomplished through a factory function. This could either be a factory at the rule-level or a factory that works for any rule

import { createRuleWithPresetOptions, noRestrictedImportsRule } from "eslint";

const noRestrictedReactImportsRule = createRuleWithPresetOptions(noRestrictedImportsRule, options)

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response

@MJez29 MJez29 added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint labels Mar 28, 2024
@mdjermanovic
Copy link
Member

Hi @MJez29, thanks for the issue!

You can "clone" a rule by adding it to your plugin and then configure it separately. For example:

import { builtinRules } from "eslint/use-at-your-own-risk";

export default [
    {
        plugins: {
            "custom": {
                rules: {
                    "no-restricted-imports": builtinRules.get("no-restricted-imports")
                }
            }
        }
    },
    {
        rules: {
            "no-restricted-imports": ["error", { paths: ["foo"] }],
            "custom/no-restricted-imports": ["warn", { paths: ["bar"] }]
        }
    }
];

Would that work for you?

@MJez29
Copy link
Author

MJez29 commented Apr 1, 2024

Yeah that'll work, thanks!

@MJez29 MJez29 closed this as completed Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Archived in project
Development

No branches or pull requests

2 participants