[import/no-extraneous-dependencies]: better support for monorepos

[armano2]

This is a feature request

Its possible that this is just an issue in documentation as i didn't dig to source code yet

packageDir option should take into account yarn workspaces or lerna projects if present to ease usage of for large monorepo projects

this can be also achieved by allowing us to use regexp / glob to find roots

Context

i recently noticed that in some of projects that I'm using, has this rule configured incorrectly,
user has to provide paths to all package.json configs even if he is working on monorepos

example of projects that can benefit from this change (big one that i worked on):

• https://github.com/typescript-eslint/typescript-eslint
  • uses yarn workspaces and lerna projects
• https://github.com/babel/babel
  • has lerna projects
• https://github.com/facebook/jest
  • uses yarn workspaces and lerna projects

Note that all of those projects currently have invalid configuration for this rule,
unless i misread documentation and package.json files are resolved relatively to parsed file

---

related comments/issues:

• import/no-extraneous-dependencies globs doesn't work on deeper folder tree (monorepo) #1302 (comment)
• may be solved by fix: allow packageDir and readPkgUp logic to play nice #1104
• no-extraneous-dependencies not checking dependencies #1177

[armano2]

it seems that this rule is not working even with directories to package configs are provided

• setup for https://github.com/conventional-changelog/commitlint

const path = require('path');
const fs = require('fs');

const isDirectory = source => fs.lstatSync(source).isDirectory();
const getDirectories = source =>
	fs
		.readdirSync(source)
		.map(name => path.join(source, name))
		.filter(isDirectory);

module.exports = {
	root: true,
	plugins: ['@typescript-eslint', 'jest', 'import'],
	env: {
		es6: true,
		node: true
	},
	parser: '@typescript-eslint/parser',
	parserOptions: {
		sourceType: 'module',
		ecmaVersion: 11,
		ecmaFeatures: {
			jsx: false
		}
	},
	extends: [
		'eslint:recommended',
		'prettier',
		'plugin:import/typescript'
	],
	rules: {
		// disallow non-import statements appearing before import statements
		'import/first': 'error',
		// Forbid import of modules using absolute paths
		'import/no-absolute-path': 'error',
		// disallow AMD require/define
		'import/no-amd': 'error',
		// Forbid the use of extraneous packages
		'import/no-extraneous-dependencies': [
			'error',
			{
				packageDir: [
					__dirname,
					...getDirectories(path.join(__dirname, '@packages')),
					...getDirectories(path.join(__dirname, '@commitlint')),
					...getDirectories(path.join(__dirname, '@alias'))
				]
			}
		],
		// Forbid mutable exports
		'import/no-mutable-exports': 'error',
		// Prevent importing the default as if it were named
		'import/no-named-default': 'error',
		// Prohibit named exports
		'import/no-named-export': 'off', // we want everything to be a named export
		// Forbid a module from importing itself
		'import/no-self-import': 'error',
		// Require modules with a single export to use a default export
		'import/prefer-default-export': 'off' // we want everything to be named
	},
	overrides: [
		{
			files: ['*.ts'],
			parser: '@typescript-eslint/parser',
			extends: [
				'plugin:@typescript-eslint/eslint-recommended',
				'plugin:@typescript-eslint/recommended',
				'prettier/@typescript-eslint'
			],
			rules: {
				'@typescript-eslint/no-unused-vars': 'off',
				'@typescript-eslint/no-use-before-define': 'off',
				'@typescript-eslint/no-explicit-any': 'off',
				'@typescript-eslint/explicit-function-return-type': 'off',
				'@typescript-eslint/no-var-requires': 'off',
				'@typescript-eslint/no-inferrable-types': 'off',
				'@typescript-eslint/no-non-null-assertion': 'off',

				// TODO: enable those rules?
				'no-empty': 'off',
				'no-var': 'off'
			}
		},
		{
			files: ['*.test.ts', '*.test.js'],
			env: {
				jest: true
			},
			extends: ['plugin:jest/recommended'],
			rules: {
				'@typescript-eslint/no-explicit-any': 'off',
				'@typescript-eslint/no-var-requires': 'off',
				'import/first': 'off'
			}
		}
	]
};

[armano2]

from my tests it seems that this rule does not work at all for monorepos even if packageDir is set

[ljharb]

A PR with failing test cases would be appreciated.

[armano2]

A PR with failing test cases would be appreciated.

not possible ATM, as there is no way to run this project on windows

---

from my tests it seems to be related to yarn, from what i understund this rule require that package is installed within node_modules, in case of yarn that may be not a case as it can be hoisted to root node_modules

[armano2]

@ljharb https://github.com/armano2/test-eslint-monorpo

---

it seems that this issue is present only on windows machines

you can find output at:

• https://github.com/armano2/test-eslint-monorpo/actions/runs/36409848

[paztis]

Hi all
I've create a PR (#1696) to correctly Work with monorepo.
It does the internal / external detection on the resolved moduelpath instead of the of the hypothetic name.
This means it also correctly work with animy resolver, because it only consider the resolved path. It resolves a lot of problematic cases.

But I've no news about the merge of it...

You can try it from my branch if you want.

[mikestopcontinues]

I got it working while looking at this thread. Given...

<root>
- packages
- - app
- - jest-config // and deps!
- - eslint-config

<root>/packages/eslint-config/index.js

// import

const path = require('path');

// export

module.exports = {
  root: true,

  extends: [
    './preset/eslint.js',
    './preset/babel.js',
    './preset/import.js',
    // etc...
  ],

  overrides: [
    {
      files: ['**/*.test.{js,ts,jsx,tsx,mjs}'],

      extends: [
        './preset/jest.js',
        './preset/jest-formatting.js',
      ],

      rules: {
        'import/no-extraneous-dependencies': [
          'error',
          {packageDir: [process.cwd(), path.resolve(__dirname, '../jest-config')]},
          // resolves to [<root>/packages/app, <root>/packages/jest-config]
        ],
      },
    },
  ],
};

<root>/packages/app/lib.test.js

import {advanceTo, clear} from 'jest-date-mock';

Despite the fact that jest-config depends on jest-date-mock, plugin-import no longer throws an error.

[armano2]

I got it working while looking at this thread. Given...

yes, that correct, some of issues got solved, since i created this ticket,
we do have consistent results between windows and linux machines

https://github.com/armano2/test-eslint-monorpo/actions/runs/471003942

[ljharb]

@armano2 does that mean this can be closed? which are the outstanding items?

[armano2]

yes it's look like issue with monorepos has been solved.

from my tests this is working fine now on win/linux/mac machines,

https://github.com/conventional-changelog/commitlint/blob/75b67b8fb7fc4df21267b98f0c9daeeb1130b824/.eslintrc.js#L31-L36

	// Forbid the use of extraneous packages
	'import/no-extraneous-dependencies': [
		'error',
		{
			devDependencies: ['**/*.test.js', '**/*.test.ts'],
		},
	],

[paztis]

Change is published in npm ?

[armano2]

@paztis i tested it on version from npm

[paztis]

packageJson version is still to 2.22.1 (4 months ago publlished)
Npm page still point to version 2.22.1
Npm semver also (https://semver.npmjs.com/)

Where do you sere it on npm ?

[ljharb]

@paztis they're saying that published version works for them.

[armano2]

@paztis they're saying that published version works for them.

exactly, in current version of package, issues reported by this tikcet has been solved, most likely there are other issues related to monorepos, but they are not related to this ticket

in current implementation you can but not have to specify packageDir to make it work with monorepos, and issue related to incositency between linux and windows machines has been resolved

https://github.com/armano2/test-eslint-monorpo/actions/runs/471003942