Enhancement: Allow no-extraneous-dependencies to check internal Imports

[narthollis]

In our monorepo I have setup that anything prefixed with out npm-scope is marked as internal, ie. ^@scope-.*/. This is working really well for us.

We are now at a stage where we are going to need to publish some of our packages to be consumed in other projects/repos. Due to this I now wish to use no-extraneous-dependencies to ensure that everyone is rembering to include this internal package references.

I did consider doing this by removing the internal regex, but this then effects the import sortinng (as we keep our internal deps grouped seperatly to our external deps - thanks to the order rule)

What I would like to see is to add an option to no-extraneous-dependencies to allow it to check internal imports based on a regex (either the internal regex or a newly defined on the rule config) as well as external.

I would be happy to work on a PR to add this option.

[ljharb]

You can already use eslint's overrides feature to target a configuration by globs. Is this sufficient?

[narthollis]

I'm not quite sure how that would work here.

I think what I am after is a bit of 'have my cake an eat it too'

What I am looking for is that mono-repo based deps to be considered as internal for import/order and external in no-extraneous-dependencies

The more i think about this request, the more I realise its a solution to a very specific problem - and that there may be a better broader solution.
Perhaps a better enhancement request would be to introduce a new (sub)-type of external import. This would make it relatively easy then to update various different rules to have appropriate behaviour, and allow for an easier to follow configuration. The order rule would be able to use this new type as an additional group and adding config to no-extraneous-dependencies would also be more strait forward.

[ljharb]

What I’m suggesting is instead of seeking to dynamically configure the rules for certain paths, to use eslint’s overrides creature to statically provide a config for different paths.

[narthollis]

I'm not quite sure how that would solve this sorry.
Any given package in the repo could (and mostly do) reference other packages in the repo.

All of our in-repo packages are prefixed with @scope- and for the sake of import ordering I configured import/internal-regex: ^@scope-.*/.
This had the side effect that no-extraneous-dependencies now ignores these imports as they are internal instead of external

What I would like to do is to keep the import ordering behaviour that I already have, but also have these 'internal' deps checked.

I am not quite sure how I could achieve this using esltint overrides - as wouldn't these overrides change the internal behaviour for all imports in the files effected by the overrides? Thereby changing the resulting order in the effected files due to the import type changing?

[ljharb]

You can override for any file in @scope/**, so that internal-regex is not configured that way.

[narthollis]

So I have packages (and files) in @scope/** that depend on other packages in@scope/**

What I am wanting to do is ensure these are all explicitly referenced in their relevant package.json files rather than relying on the implicit references that work thanks to yarn work spaces and lerna.

(eg. @scope/a requires @scope/b and @scope/c requires @scope/a also) - currently I have these @scope/** references marked as internal imports so that they get grouped with the other internal imports (such as src/foo/bar - this has proven helpful for new devs getting used to our monorepo - this all being said, if there is not a good way to get both worlds i'll remove the internal regex and ware the fallout because its more important these references exist when we start sharing our mono repo packages with other teams)

(with the thinking mentioned above tho, i think my ideal would would be to treat these 'external but in mono-repo' deps as as a not-quote-external-external - such they get checked by no-extrusions=dependancies but their sort order is not one with actual external deps when it comes to the import/order rule)

[mikestopcontinues]

I'm having the exact opposite problem. Maybe we can merge our configs?

Right now, when I lint from my project root, I'm getting this error:

/Users/msc/Code/hub/@arc/infra/cdk/node_modules/@arc/view-app/node_modules/@arc/ui/hooks/useWindowScroll.js
3:1 error '@arc/ui' should be listed in the project's dependencies. Run 'npm i -S @arc/ui' to add it import/no-extraneous-dependencies

no-extraneous-dependencies can detect that @arc/cdk must include @arc/view-app in it's deps. But it's throwing an error because @arc/cdk doesn't directly rely on @arc/ui, while @arc/view-app does.

My relevant config looks like this:

// import

const path = require('path');

const fs = require('fs-extra');
const globby = require('globby');

// vars

const cwd = process.cwd();
const root = path.resolve(__dirname, '../../../');
const lernaFile = path.resolve(cwd, './lerna.json');

const lernaGlobs = fs.existsSync(lernaFile) ? fs.readJsonSync(lernaFile).packages : [];
const pkgPaths = lernaGlobs
  .flatMap((loc) => globby.sync([loc, '!**/node_modules/**'], {onlyDirectories: true, expandDirectories: false}))
  .map((loc) => path.resolve(cwd, loc));

const pkgContainers = fs.readJsonSync(path.resolve(root, 'lerna.json')).packages
  .map((loc) => path.resolve(root, loc.replace(/(\/|^)[^/]+$/, '')));

// export

module.exports = {
  plugins: ['import'],

  settings: {
    'import/resolver': {
      'node': {
        extensions: ['.js', '.jsx'],
        paths: [cwd, ...pkgPaths],
      },
      'eslint-import-resolver-lerna': {
        packages: pkgContainers,
      },
    },
    'import/ignore': [/\.es\.js$/],
  },

  rules: {
    'import/no-extraneous-dependencies': 2
  },
};

[mikestopcontinues]

Ahh, I actually realized I had a circular dependency issue. But in the process, I discover this chestnut, which might help @narthollis

overrides: [
    {
      files: [
        '*/**/node_modules/**/*',
      ],
      rules: {
        'import/no-extraneous-dependencies': 0,
      },
    },
  ],

[tlmader]

@narthollis did you ever find a solution to this? Looking to keep all my external dependencies on the root package.json to reduce version inconsistencies, while adding internal packages as explicit dependencies so that Nx can properly build its graph.

[bdwain]

@ljharb this seems as simple as adding a config option that changes the behavior on this line https://github.com/import-js/eslint-plugin-import/blob/main/src/rules/no-extraneous-dependencies.js#L170

is that something you'd be open to if someone made a PR for it?

[ljharb]

@bdwain i agree that would work, but since eslint overrides make solving this problem as flexibly as you like, i'm not sure why it's better to add complexity to the plugin.

[bdwain]

@ljharb Can you explain in more detail how overrides solve the problem? Or give an example? I’m not really seeing it.

In my case at least (which I think is the original posters case) I want the import ordering rules (which are based on the internal regex) and the extraneous dep rules both to apply to all files. How can we add the internal regex setting and not add it for the same set of files?

I thought maybe you meant to do an override for * and in that override add the internal setting and the import order rule, but that didn’t seem to work.

It doesn’t really feel like a case for overrides because it’s not different rules per file but different settings per rule.

[ljharb]

This is certainly a confusing thread to try to page back in :-)

The point of no-extraneous-dependencies is to ensure that everything required/imported is explicitly listed in package.json. Each dependency "category" can be set to a boolean (allowed or forbidden), or, to an array of globs. The "array of globs" feature predates overrides, and we would not have added that feature if overrides had already existed (because instead of having one rule config for the entire repo, you'd have one default top-level one, and one override that always enabled, eg, dev deps by glob).

By default, if you've marked monorepo packages as "internal", then this rule will not require monorepo packages to be listed explicitly in order to require/import them. If they're not marked "internal", then the order rule won't sort them properly.

Linting anything inside a node_modules directory is an antipattern, and not a good workaround.

It sounds like the solution here is indeed to add an option to no-extraneous-dependencies that allows checking of "internal" import types. It seems like the only other type that might be desirable to check here is "internal", so we wouldn't need to worry about an enum - maybe includeInternal that defaults to false?

can you all confirm (@bdwain @narthollis @mikestopcontinues @tlmader) that this would solve your use case?

[bdwain]

@ljharb thanks for getting back so quickly. this would indeed solve my use case. and i think the boolean is fine vs an enum, but not 100% sure on all the types that we might encounter?

[bdwain]

im also happy to make a pr for this unless you were planning on it

[ljharb]

I'd love a PR once some of the others have confirmed it would work.

[narthollis]

Yeah, this sounds like it would work for me

[bdwain]

@ljharb separate thing i just noticed when looking at this code is that it explicitly ignores type imports. Is there a reason for that? I was thinking an option to enable checks on type imports would be nice too. i figure i could add that also while i'm at it.

[ljharb]

@bdwain usually because TS checks type imports for you. Is there any way to actually know the package name types come from? I don't know if the TS parser provides that.

Let's look into that as a separate PR, just in case it's impossible :-)

[bdwain]

Sounds good! I’ll work on this one first. Thanks

[mikestopcontinues]

Yup, this'd be great! Thanks!

[ljharb]

3 out of 4's pretty good, let's roll :-)

[bdwain]

PR is up #2541