willhoney7/eslint-plugin-import-helpers

Question: Why remove `builtin` in v1?

lffg opened this issue · 5 comments

lffg commented

Hi,

First of all, thanks for this awesome ESLint plugin. It is really helpful to organize my project imports! :)


My question is in about the new v1 version: why did you remove the builtin? I personally consider that option quite useful.

In regard the internal and external options, I maybe understand the reason that made you remove them. For me it seems quite tricky to differentiate an internal module that uses absolute importing and a normal external module. Was that the reason?

Hi! I'm so happy to hear this has been useful to you!

Regarding, external and internal, yes I dropped that because supporting it required attempting to resolve the imports and do things beyond looking at the actual import path string. It was a lot of work for minimal gain.

Regarding builtin, it seems unnecessary since you can replicate the exact functionality with regular expression groups. I think it's more of a special case than a "standard" group. See the migration guide on the regular expression.

I'm absolutely open to adding it back in the future if enough people want it. However, being able to still support it with a RegEx makes me lean towards keeping it out. Plus this way you can keep the list up to date and manage it yourself (I don't have to update this rule to support the next version of node, etc).

lffg commented

I see...

Thanks for the reply. :)

@Tibfib hey, just want to add my words to this. I think keeping builtin is important, otherwise we have to write absolutely the same regexp all the time.

@ezhlobo Apologies for the very late response on this.

All the time is pretty exaggerated I think. This ESLint rule isn't something you have to set up/edit every day... maybe every couple of months. In that case, copy and pasting that regexp doesn't seem to be a huge deal.

And, as I said earlier, this way requires less maintenance and updates to this rule.

Plus this way you can keep the list up to date and manage it yourself (I don't have to update this rule to support the next version of node, etc).

lffg commented

I ended up doing something like this:

const builtInModules = require('builtin-modules');

module.exports = {
  rules: {
    'import-helpers/order-imports': [
      WARN,
      {
        groups: [
          [`/^(${builtInModules.join('|')})$/`],
          // Other groups... :)
        ],
        alphabetize: { order: 'asc', ignoreCase: true },
        newlinesBetween: 'never'
      }
    ]
  }
};

The module builtin-modules returns an array containing all Node.js built in modules.