Skip to content
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

import/no-extraneous-dependencies globs doesn't work on deeper folder tree (monorepo) #1302

Closed
Hotell opened this issue Mar 18, 2019 · 11 comments

Comments

@Hotell
Copy link

Hotell commented Mar 18, 2019

Globs doesn't work on nested folders within monorepo. I'm getting lint errors within foo.test.tsx or foo.story.tsx

image

Here is my project structure:

├── packages
│   ├── common
│   │   ├── README.md
│   │   ├── __tests__
│   │   ├── dist
│   │   ├── package.json
│   │   ├── src // 👉 *.test.tsx and *.story.tsx reside here
│   │   ├── tsconfig.build.json
│   │   └── tsconfig.json
│   ├── components
│   │   ├── README.md
│   │   ├── dist
│   │   ├── package.json
│   │   ├── src // 👉 *.test.tsx and *.story.tsx reside here
│   │   ├── tsconfig.build.json
│   │   └── tsconfig.json

config:

.eslintrc.js:

module.exports = {
  root: true,
  extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended'],
  parser: '@typescript-eslint/parser',
  plugins: ['@typescript-eslint', 'import'],
  rules: {
    'import/no-extraneous-dependencies': [
      'error',
      {
        devDependencies: [
          '**/*.test.{ts,tsx}',
          '**/*.spec.{ts,tsx}',
          '**/*.story.tsx',
          '**/__tests__/**',
          '**/__mocks__/**',
          './config/**/*.js',
        ],
      },
    ],
  },
};
@ljharb
Copy link
Member

ljharb commented Mar 18, 2019

Have you set the packageDir option?

@Hotell
Copy link
Author

Hotell commented Mar 24, 2019

yup, same output

'import/no-extraneous-dependencies': [
      'error',
      {
        packageDir: [
          './packages/common/',
          './packages/components/',
          './packages/customer-registration/',
          './',
        ],
        devDependencies: [
          '**/*.test.{ts,tsx}',
          '**/*.spec.{ts,tsx}',
          '**/*.story.tsx',
          '**/__tests__/**',
          '**/__mocks__/**',
          './config/**/*.js',
        ],
      },
    ],

@Izhaki
Copy link

Izhaki commented Jan 9, 2020

@Hotell your setup (with packageDir) messes up things big time from me. I think it says to eslint:

When you work out imports in any package test against all these package files.

So imports in common will be tested against the package.json file of components - you don't want that.


Instead, remove packageDir property in the root .eslintrc.js and in each package drop this eslintrc.js:

const { join } = require('path');

module.exports = {
  rules: {
    'import/no-extraneous-dependencies': [
      'error',
      // Use package.json from both this package folder and root.
      { packageDir: [__dirname, join(__dirname, '../../')] }
    ]
  }
};

This one says:

When you work out the imports in this folder, also account for the root package.json

@ljharb
Copy link
Member

ljharb commented Jan 11, 2020

@Hotell can you try again with v2.20.0 and confirm if the problem still exists?

@hedgepigdaniel
Copy link

yup, same output

'import/no-extraneous-dependencies': [
      'error',
      {
        packageDir: [
          './packages/common/',
          './packages/components/',
          './packages/customer-registration/',
          './',
        ],
        devDependencies: [
          '**/*.test.{ts,tsx}',
          '**/*.spec.{ts,tsx}',
          '**/*.story.tsx',
          '**/__tests__/**',
          '**/__mocks__/**',
          './config/**/*.js',
        ],
      },
    ],

Trouble with that is if packages/components/' has a dependency on something, that dependency needs to be either in packages/components/package.json or in ./package.json. It doesn't work if the dependency is in some other package (e.g. packages/customer-registration/package.json).

In this setup it is satisfied by a dependency in any package in the monorepo, not the package where the dependency is used.

@ljharb
Copy link
Member

ljharb commented Jan 30, 2020

if packages/components/' has a dependency on something, that dependency needs to be either in packages/components/package.json or in ./package.json. It doesn't work if the dependency is in some other package (e.g. packages/customer-registration/package.json).

This is correct - both how it should work, and how it works with node's resolution algorithm.

Are you saying it's not working that way? Or that you want it to be "satisfied by a dependency in any package in the monorepo"?

@hedgepigdaniel
Copy link

I realised aht I'm saying is just what ws said in #1302 (comment)

Except I tried using packageDir separately in each subpackage (specifying only that package and the root package) but this resulted in the rule not generating any warnings at all.

@hedgepigdaniel
Copy link

I found good alternative using eslint overrides: #1214 (comment)

@paztis
Copy link

paztis commented Apr 9, 2020

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.

@fernandopasik
Copy link
Contributor

@ljharb would this one be closed as well as #1174 ?

@ljharb
Copy link
Member

ljharb commented Jan 30, 2021

Seems reasonable. Please file a new issue if the next release does not address any of these issues.

@ljharb ljharb closed this as completed Jan 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants