-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Enhancement: Monorepos and no-extraneous-dependencies #1174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
We're seeing same issue in our code base. Any suggestion how to fix it? |
Any updates on this issue? |
For anyone in need, I have figured out some sort of workaround using overrides. Repository:
From
Since we know the package paths (from package.json 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],
}; |
I'm bumping on to this too. This could be solved if the |
Hi all But I've no news about the merge of it... You can try it from my branch if you want. |
hi @paztis, do you have a published temporary module with the changes? |
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 <[email protected]> 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>
.
|
@paztis I've reviewed your PR and asked you some questions; it needs updating. |
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 <[email protected]> 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>
.
|
When is this planned to be released? |
I do the code review fixes and sync with master. |
Hi guys! Any updates? |
I'm still waiting my PR is approved. It might be better to post on the PR page for info about it: |
¿Hi @benmosher @paztis @ljharb Any updates? Thank in advance... |
@fkoner as you can see, the linked PR has been merged, but not yet released. As such, I'll close this. |
The PR has been merge 2 months ago but there's still no release on it. |
When I resolve #1986. |
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 |
Yes, in >= v2.23.0 802ce7d |
@ljharb But the documentation still seems to imply that only the nearest |
We still face this error when using |
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. |
A common configuration for monorepos, popularized by Lerna and Yarn workspaces, is
The expected behaviour (especially for devDependencies) is that modules required in
a/index.js
must be specified in eithera/package.json
or inroot/package.json
; specifying it inb/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:
./package.json
,../package.json
, ... until the current working directory (or project root) is reached.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 orfoo
is found. Even if an intermediate level contains anode_modules
directory, if it does not contain the modulefoo
, node continues the search.The text was updated successfully, but these errors were encountered: