Enhancement: Monorepos and no-extraneous-dependencies

[aravindet]

A common configuration for monorepos, popularized by Lerna and Yarn workspaces, is

root
+- package.json
+- packages
  +- a
  |  +- package.json
  |  +- index.js
  +- b
  |  +- package.json
  |  +- index.js

The expected behaviour (especially for devDependencies) is that modules required in a/index.js must be specified in either a/package.json or in root/package.json; specifying it in b/package.json should not matter.

Currently, there does not seem to be a way to accomplish this, as the packageDir option is relative to the project root and is unaffected by the location of the file is being checked.

I propose that the default behaviour (when no packageDir is specified) be the following:

• From the directory of the file being checked, look for ./package.json, ../package.json, ... until the current working directory (or project root) is reached.
• Merge all the discovered deps, devDeps, peerDeps and optDeps
• Check imports against this.

This matches node's module resolution algorithm, which tries to resolve require(foo) in ./node_modules/foo, ../node_modules/foo, ../../node_modules/foo, etc. until it reaches the filesystem root or foo is found. Even if an intermediate level contains a node_modules directory, if it does not contain the module foo, node continues the search.

[alexindigo]

We're seeing same issue in our code base. Any suggestion how to fix it?

[nmccready]

This might help https://github.com/Dreamscapes/eslint-import-resolver-lerna

[dawidk92]

Any updates on this issue?

[marcinlanger]

For anyone in need, I have figured out some sort of workaround using overrides.
The project is using a mono repo utilizing yarn workspaces, but this should work with lerna and other tools as well.

Repository:

root
+- package.json (workspaces: ["packages/*"])
+- .eslintrc.js
+- packages
    +- a
        +- package.json
    +- b
        +- package.json

From no-extraneous-dependencies documentation:

Also there is one more option called packageDir, this option is to specify the path to the folder containing package.json.

Since we know the package paths (from package.json workspaces or from lerna.json) and .eslintrc.js is in the root, we can use this to loop through package paths and add an override for each of them, adding the root path and the package path to packageDir.

Here is an example config. Obviously you could leverage the glob patterns from your mono repo configuration instead of hardcoding one packages folder to make this work with more complex folder structures.

const { resolve } = require('path');
const { readdirSync, lstatSync } = require('fs');

const PACKAGE_DIR = 'packages/'; // this could be replaced utilizing the globs in package.json's "workpackges" or from the lerna.json config

// get files in packages
const noExtraneousOverrides = readdirSync(resolve(__dirname, PACKAGE_DIR))
	// filter for non-hidden dirs to get a list of packages
	.filter(
		entry =>
			entry.substr(0, 1) !== '.' && lstatSync(resolve(__dirname, PACKAGE_DIR, entry)).isDirectory(),
	)
	// map to override rules pointing to local and root package.json for rule
	.map(entry => ({
		files: [`${PACKAGE_DIR}${entry}/**/*`],
		rules: {
			'import/no-extraneous-dependencies': [
				'error',
				{
					devDependencies: true,
					optionalDependencies: false,
					peerDependencies: false,
					packageDir: [__dirname, resolve(__dirname, PACKAGE_DIR, entry)],
				},
			],
		},
	}));

module.exports = {
	extends: 'eslint-config-acme',
	overrides: [...noExtraneousOverrides],
};

[dietergeerts]

I'm bumping on to this too. This could be solved if the packageDir option would allow a glob, so we can define all our packages with 1 string, instead of needing to list them all.

[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.

[roastlechon]

hi @paztis, do you have a published temporary module with the changes?

[paztis]

Yes, under @paztis/eslint-plugin-import But it is unusable because of Airbnb the always takes original one. As both will be present with the same rules names, we are not able to really use it... I can use it with symlinks for now Le jeu. 25 juin 2020 à 20:27, Noel Madali <notifications@github.com> a écrit :

…

hi @paztis <https://github.com/paztis>, do you have a published temporary module with the changes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1174 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABIKMMI575MGJIKKKX3SOIDRYOJIPANCNFSM4FXXFH3Q> .

[ljharb]

@paztis I've reviewed your PR and asked you some questions; it needs updating.

[paztis]

Yes sorry Was busy With my job. I will finish to treat the review feedbacks begining of next week Le jeu. 25 juin 2020 à 20:39, Jordan Harband <notifications@github.com> a écrit :

…

@paztis <https://github.com/paztis> I've reviewed your PR and asked you some questions; it needs updating. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1174 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABIKMMKL5BHETB4NMYANPBTRYOKWZANCNFSM4FXXFH3Q> .

[francisminu]

When is this planned to be released?

[paztis]

I do the code review fixes and sync with master.
Just need your last status now

[egemon]

Hi guys! Any updates?

[paztis]

I'm still waiting my PR is approved.
The repo authors are not replying since a moment.

It might be better to post on the PR page for info about it:
#1696

[fkoner]

¿Hi @benmosher @paztis @ljharb Any updates?

Thank in advance...

[ljharb]

@fkoner as you can see, the linked PR has been merged, but not yet released. As such, I'll close this.

[paztis]

The PR has been merge 2 months ago but there's still no release on it.
Any idea when it will be available ?

[ljharb]

When I resolve #1986.

[marcospgp]

It seems like there has been a lot of activity mentioning this issue, has it made it into a release yet?

I am seeing an error when importing from a package.json that is one directory above the nearest one, so it seems not, but maybe I should update my yarn.lock.

[ljharb]

Yes, in >= v2.23.0 802ce7d

[marcospgp]

@ljharb But the documentation still seems to imply that only the nearest package.json will be looked at 🧐 is it outdated?

https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-extraneous-dependencies.md?plain=1#L6

[beanaroo]

Yes, in >= v2.23.0 802ce7d

We still face this error when using v2.27.5 with npm workspaces when dependencies are declared at the root package.json level. Is this worth opening a separate issue?

[awdyson]

I've got a main file + file-per-package setup like this:

/* src/package/some-package */

// @ts-check
/** @type {import('eslint-define-config').ESLintConfig} */
module.exports = {
  extends: ['../../.eslintrc.cjs'],
  parserOptions: {
    project: ['./tsconfig.json'],
  },
  rules: {
    'import/no-extraneous-dependencies': [
      1,
      {
        devDependencies: false,
        includeInternal: false,
        includeTypes: false,
        packageDir: ['.', '../..'], // <--- the key addition
      },
    ],
  },
};

Plus this seemed to be the easiest way to set the project for TS to lower processing time.