import/no-extraneous-dependencies and import/order false postives since version 2.23

[mlbiche]

Since the update to version 2.23, we have a lot of false positives in our project being part of two categories :

• error 'rxjs/operators' should be listed in the project's dependencies. Run 'npm i -S rxjs/operators' to add it import/no-extraneous-dependencies : rxjs/operators is a sub-module of rxjs so I don't have to add it to project dependencies.
  *error `@models/partner/partner-id` import should occur before import of `@models/partner/partner` import/order : I can fix these issues, but it seems that the order has changed between two versions and I don not understand why and if it is on purpose ? I can not find mention to this in the changelogs.

Cheers 🤙

[tarlepp]

I'm having this same issue with rxjs/operators any solution to fix that?

[ljharb]

cc @paztis, #1696 may have caused this first issue.

[ljharb]

@mlbiche for the order issue, can you provide the exact code it's warning on?

[mlbiche]

@ljharb Here it is :

import { Partner } from '@models/partner/partner';
import { PartnerId } from '@models/partner/partner-id';

It is doing the same whenever there are two files at the same path with a common prefix. Before, the shortest one was considered first.

[tarlepp]

Updated to "eslint-plugin-ngrx": "1.23.0" and I 'm still getting;

/app/src/app/app.component.ts
  7:1  error  'rxjs/operators' should be listed in the project's dependencies. Run 'npm i -S rxjs/operators' to add it  import/no-extraneous-dependencies

app.component.ts:7;

import { distinctUntilChanged, filter, map, take } from 'rxjs/operators';

[ljharb]

#2071 should fix the ordering issue.

[paztis]

Is the defect still present ?
EWHat is the resolver plugin you use ? I don't encounter the issue on my side

[paztis]

ah ok I see the problem.
With the new mechanism, we search for hte fist packageJson ancestor

the problem is rxjs /operators/package.json exists with this content

{
  "name": "rxjs/operators",
  "typings": "./index.d.ts",
  "main": "./index.js",
  "module": "../_esm5/operators/index.js",
  "es2015": "../_esm2015/operators/index.js",
  "sideEffects": false
}

So it isconsidered as an independent module, what is not the case...
It is an error of the rxjs developers I think

What can we do for this ?

[clydin]

That is the package.json for a secondary entry point of the package. Until the exports field becomes more widely supported these files are unfortunately going to be around.

To find the root package.json for a particular package, the node_modules directory could to be used in the check.
For example:

• node_modules/<unscoped_package>/package.json
• node_modules/@<scope>/<scoped_package>/package.json

Alternatively, the package.json could be read and validated for all required fields but that would probably be more expensive. There is no version in the above package.json, for example.

[ljharb]

@paztis package.json files are only the root of an installed project when ../ (relative to that file) is named "node_modules".

[paztis]

No, this is not true. I do this recursive search explicitely to support the. Monolithic repositories (multiple packages in one repository).
If we do like you said, we will always found a parent package.json my hat is totally unrelated with the package.

Standards are 1 package.json for each delivreable

[paztis]

In case we of monorepo, what is possible is to resolve the import file without resolving the symbolic links. Potentially this may also us to check the node_module path. We still have the case of scoped modules to verify

[tarlepp]

{
  "name": "rxjs/operators",
  "typings": "./index.d.ts",
  "main": "./index.js",
  "module": "../_esm5/operators/index.js",
  "es2015": "../_esm2015/operators/index.js",
  "sideEffects": false
}

Note that this in 6.x branch and not in main (7.x) anymore - not sure if this error occurs with 7.x version - cannot test that because I'm using RxJs with Angular and it doesn't support RxJs 7.x at this time.

[paztis]

solution I propose in #2072 might resolve your problem. It's will be a little less strict on the imports but might be enough for these cases

[ljharb]

@paztis i said an installed project. In other words, if you're inside a node_modules directory, then you're not in a monorepo, you're an npm-installed package, and the rule I described applies.

[paztis]

I really understand your problem.
But what you misunderstood is that in a mini repo, under node_modules you have all the modules of you monorepo in symlink.

So when you resolve the filename of the imports, the resolver can give you the real path of the file that is not under node_modules.
This is the principle of monorepos

And as I can it control the resolvers as they are plugins of eslit-import (lot of plugins exist on nom), I won't just check if I'm under node module or not

[ljharb]

@paztis yes, but the symlink will be realpathed, so the resulting paths won't be under node_modules.

[ljharb]

Either way, "supporting a monorepo" is of zero importance compared to "supporting normally installed dependencies", if there's a conflict between the two.

[paztis]

Sorry but the only non standard thing here is the rocks module that produces abnormal package.json in its subfolders.

Now I've propose you a solution that will resolve your errors. So do you want it or not ?

[ljharb]

Nested package.jsons aren't abnormal, they are an explicitly encouraged pattern with node's native ESM support.

I'll take a look at your comment in #2072.

[paztis]

To detect the module package from one package file, we're using read-pkg-up, downloaded 38 228 565 times on npm.
It recursively move up until it founds a package.json
So it means everyone that use this module is not correctly discovering the packages ?

[paztis]

Esm in node is really recent. Monorepo are older.
Need to found one solution to make both cohabit

[ljharb]

Yes, if they're using read-pkg-up and stopping at the first package.json AND using it as the project root, then they all have this bug.

Nested package.jsons aren't recent; node has always supported nested package.json files with a "main". The issue is that the closest package.json hasn't ever been guaranteed to be the package root, so that's the bug that needs fixing.

[piranna]

Yes, if they're using read-pkg-up and stopping at the first package.json AND using it as the project root, then they all have this bug.

From its readme:

Read the closest package.json file

To me, that's not a bug, but instead the expected behaviour.

[ljharb]

Of that package, absolutely. However, anyone naively using it to find the root package.json for a package is what has a bug.

[piranna]

But, how to detect the actual package root?

[ljharb]

@piranna there isn't a robust way for things not inside a node_modules folder. There's reliable signals - if it has "exports", or "publishConfig", or "private", I'd call it top-level. If the repo has a ".git" directory, it's probably top-level. It'd be a heuristics-based approach.

[JFGHT]

I am having this very same problem with Firebase imports.

[paztis]

I've proposed a fix for extraneous-dependencies in PR #2097

[Temez1]

Still happens with firebase

"eslint-plugin-import": "^2.24.2",

[latipun7]

Still happens with firebase

"eslint-plugin-import": "^2.24.2",

did you list @firebase/auth in your project dependencies or devDependencies? If not, then it's expected result. Even if you have @firebase/auth in your node_modules as part of the another module's dependencies.

This issue happened mainly for sub module or sub files of module detected as individual module which is already resolved. While @firebase/auth is individual module because the module name is scoped to @firebase.
For example rxjs is individual module that exported operators.js in that module. So we can import rxjs/operators.js or rxjs/operators which is valid as rxjs as dependencies.

[Temez1]

Oh, I didn't know about the scope thing at all. And I don't know why firebase is using separate modules as its own packages when it says it's not supposed to be used directly :D

Anyways, all good when I removed the "@" and used firebase/auth instead.

Thanks for the info.