javierbrea / eslint-plugin-boundaries

Eslint plugin checking architecture boundaries between elements
MIT License
473 stars 9 forks source link

Feature request: [TypeScript] A setting to ignore `import type` #298

Closed jeffreykwan closed 10 months ago

jeffreykwan commented 11 months ago

Is your feature request related to a problem? Please describe. TypeScript has a feature that allows you to import types and I would like to have a setting to ignore these types of boundary violations

// in moduleB
import ModuleA from "./moduleA"
import type { ModuleAType } from "./moduleA" // this should not error

Describe the solution you'd like In eslint settings:

settings: {
  allowTypeImports: true
}

When using the typescript-eslint/parser, the node we receive from dependencyRule has the property importKind that we can check if it's importing a type or a value.

Here's a diff of the change that would be required (still have to add the setting and check that value)

diff --git a/src/rules/element-types.js b/src/rules/element-types.js
index 0582fb9..10fdf89 100644
--- a/src/rules/element-types.js
+++ b/src/rules/element-types.js
@@ -46,6 +46,10 @@ module.exports = dependencyRule(
   },
   function ({ dependency, file, node, context, options }) {
     if (dependency.isLocal && !dependency.isIgnored && dependency.type && !dependency.isInternal) {
+      if (node.importKind === "type") {
+        return;
+      }
+
       const ruleData = elementRulesAllowDependencyType(file, dependency, options);
       if (!ruleData.result) {
         context.report({

If there is interest in this, I'll be happy to submit a PR

javierbrea commented 10 months ago

Hi @jeffreykwan , First of all, thanks for the suggestion 😃 . I think that ignoring them may won't be what every plugin's users would expect, and it would be a breaking change. So, please let me think about how to add this feature as an option to the element-types rule, so it can be used even to define different rules for type imports.

javierbrea commented 10 months ago

@jeffreykwan, This may be solved by adding a new importKind option to rules options, so you can decide to which type of imports the rule should apply. It could be added to element-types, entry-point and external rules. For example, the new option in the element-types rule would look like:

{
  "rules": {
    "boundaries/element-types": [2, {
        // disallow all local imports by default
        "default": "disallow",
        "rules": [
          {
            // from helper elements
            "from": ["helpers"],
            // allow importing helper elements
            "allow": ["helpers"],
           // This rule applies to all type of imports
            "importKind": "*",
          },
          {
            // from module elements
            "from": ["modules"],
            // allow importing helper elements
            "allow": ["helpers"],
           // This rule applies when importing values
            "importKind": "value"
          },
          {
            // from module elements
            "from": ["modules"],
            // allow importing types from helper and module elements
            "allow": ["helpers", "modules"],
           // This rule applies when importing types
            "importKind": "type"
          },
        ]
      }
    ]
  }
}

By default, the new option would have the "*" value, which should apply to all type of imports, so it won't be a breaking change. And allowed values for it would be "type", "value", or "typeof", according to the Babel documentation. The value for the option could be also defined using an array of allowed values, or micromatch patterns, in order to keep consistency with the rest of options.

This way, you could easily ignore type imports by adding "importKind": "value" to your current rules, but you would also be able to define rules for type imports.

youssefm commented 8 months ago

Thanks for adding this feature @javierbrea. It just came in handy to allow us to import types while disallowing "real" imports.