nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.58k stars 2.36k forks source link

Eslint rule `enforce-module-boundaries` false positive #15957

Closed JesusTheHun closed 1 year ago

JesusTheHun commented 1 year ago

Current Behavior

import * as Sentry from '@sentry/node';
// ^ ESLint: Projects should use relative imports to import from other files within the same project. Use "./path/to/file" instead of import from "@sentry/node"(@nrwl/nx/enforce-module-boundaries)

// EDIT : actually, even a simple import can cause an issue
import { hotjar } from "react-hotjar";
// ^ Same error

Expected Behavior

No error

GitHub Repo

No response

Steps to Reproduce

Import any package with a namespace

Nx Report

Node : 18.13.0
OS   : darwin arm64
npm  : 8.19.3

nx                      : 15.8.9
@nrwl/js                : 15.8.9
@nrwl/jest              : 15.8.9
@nrwl/linter            : 15.8.9
@nrwl/workspace         : 15.8.9
@nrwl/cli               : 15.8.9
@nrwl/cypress           : 15.8.9
@nrwl/devkit            : 15.8.9
@nrwl/esbuild           : 15.8.9
@nrwl/eslint-plugin-nx  : 15.8.9
@nrwl/node              : 15.8.9
@nrwl/react             : 15.8.9
@nrwl/tao               : 15.8.9
@nrwl/vite              : 15.8.9
@nrwl/webpack           : 15.8.9
typescript              : 4.9.5

Failure Logs

No response

Additional Information

No response

meeroslav commented 1 year ago

Thank you @JesusTheHun for reporting this.

This seems to be coming back but we can never reproduce it. Can you provide a minimal repo with this error? Please also try to reinstall your dependencies and let me know if the error persists.

  1. rm -rf node_modules
  2. npm ci/yarn --frozen-lockfile/pnpm install --frozen-lockfile
  3. Run lint again
rmjwilbur commented 1 year ago

New to Nx but getting this as well on any imports/exports from @aws-amplify, ie:

import { Auth } from '@aws-amplify/auth';

and

export { RestAPI as api } from '@aws-amplify/api-rest';

rmjwilbur commented 1 year ago

Hope this example works for you: https://github.com/petconsole/mini-repo-1/blob/main/libs/pure-fe-api/src/ourYupSchema.ts

I'm also getting the issue on imports from yup, react, redux, etc.

jogelin commented 1 year ago

@meeroslav I have the same issue when trying to switch from yarn to pnpm:

ESLint: Transitive dependencies are not allowed. Only packages defined in the "package.json" can be imported(@nrwl/nx/enforce-module-boundaries)

This is happening only on 2 dependencies: '@auth0/auth0-angular' and '@cumul.io/ngx-cumulio-dashboard'

After debugging, it seems that the rule is correct but the nxdeps.json contains wrong values, for example:

...
    "npm:@auth0/auth0-angular@2.0.2(@angular/common@15.2.5)(@angular/core@15.2.5)(@angular": {
      "type": "npm",
      "name": "npm:@auth0/auth0-angular@2.0.2(@angular/common@15.2.5)(@angular/core@15.2.5)(@angular",
      "data": {
        "version": "router@15.2.5)",
        "packageName": "@auth0/auth0-angular@2.0.2(@angular/common@15.2.5)(@angular/core@15.2.5)(@angular",
        "hash": "sha512-MdZakp0M3NbRd0HsNHzML9XPYyDPIF6oq6NCsn+dY1DKizQee6MP90PrezXDDQ3JpRkgbkVRL0cjEu7HHA8bug=="
      }
    },
    "npm:@auth0": {
      "type": "npm",
      "name": "npm:@auth0",
      "data": {
        "version": "auth0-spa-js@2.0.4",
        "packageName": "@auth0",
        "hash": "sha512-NgD6Fkm5Xnbned1VjhEW8irm1/Y8AWtdSLexgLXNwwBVgfnYicCVBu8w75m2t+4QljXkTcI7IXVEFhVmxMuxaA=="
      }
    },
...

As you can see, the version name is wrong. Hope this can help!

nx report:

 >  NX   Report complete - copy this into the issue template

   Node : 18.13.0
   OS   : darwin x64
   pnpm : 8.1.1

   nx                      : 15.9.1
   @nrwl/js                : 15.9.1
   @nrwl/jest              : 15.9.1
   @nrwl/linter            : 15.9.1
   @nrwl/workspace         : 15.9.1
   @nrwl/angular           : 15.9.1
   @nrwl/cli               : 15.9.1
   @nrwl/cypress           : 15.9.1
   @nrwl/devkit            : 15.9.1
   @nrwl/eslint-plugin-nx  : 15.9.1
   @nrwl/node              : 15.9.1
   @nrwl/nx-plugin         : 15.9.1
   @nrwl/storybook         : 15.9.1
   @nrwl/tao               : 15.9.1
   @nrwl/web               : 15.9.1
   @nrwl/webpack           : 15.9.1
   typescript              : 4.9.5
   ---------------------------------------
   Community plugins:
   @auth0/auth0-angular            : 2.0.2
   @cumul.io/ngx-cumulio-dashboard : 3.2.0
   @ngneat/transloco               : 4.2.6
   @ngrx/component                 : 15.4.0
   ng2-charts                      : 4.1.1
   single-spa-angular              : 8.1.0
   @compodoc/compodoc              : 1.1.18
   @ngneat/spectator               : 14.0.0
   @storybook/angular              : 6.5.16
   ---------------------------------------
   Local workspace plugins:
     @grizzly/nx-plugin
meeroslav commented 1 year ago

@jogelin, what you see is an issue with missing support for pnpm v8 (or lock file v6), which was merged two weeks ago but is only available since 16.0.0-beta.0.

meeroslav commented 1 year ago

@rmjwilbur, unfortunately, I could not reproduce this on a fresh clone of your repo.

Screenshot 2023-04-18 at 12 20 07

I'm closing this error as not reproducible. If someone can produce a repo where this can be reproduced on a clean state I will gladly re-open it.

sophher commented 1 year ago

We faced the same issue after an eslint update. We solved the original problem (we had both a lazy and a static import of the same module), but the linter still complained. After deleting the NX cache the lint was gone.

balintbrews commented 1 year ago

@meeroslav I might be wrong — I'm also new to Nx — but your screenshot shows that you only installed dependencies from the root package.json file. The sample repo @rmjwilbur prepared has a project-level package.json inside libs/pure-fe-api. I believe if you install that, too, you will get the error. Interestingly, ESLint doesn't complain about the missing module in case you skip installing that, which makes your linting pass. I can reproduce this behavior with my own repository.

the1979 commented 1 year ago

We have the same issue - dependencies listed in a project-level package.json only (as @balintk points out), are not taken into consideration by the rule. Anything imported from those packages fails linting. Have updated to nx 16.3.2 and @nx/eslint-plugin 16.3.2 - still persists. @meeroslav could we get this reopened?

meeroslav commented 1 year ago

@balintk you are correct and that's how it should be done.

Unfortunately, that repo behaves like a workspace (having a nested package.json) but without a specification in the root package.json which leads to packages missing in the root lock file and out graph not discovering it.

The proper way to do it would be to add something like this to the root package.json and remove the nested lock file:

  "workspaces": [
    "libs/*" // or be more explicit if only some of the projects have a package.json
  ]

Doing this would still install yup from the nested project, but it would keep it in the root node_modules. Additionally, you would install all dependencies by running a single npm install from the root without the need to go into each of the projects. And most important, our lock file parser would properly finds that dependency.

@the1979 try following the steps above to check if this fixes the issue for you.

However, the error you receive when the solution is in a bad state is incorrect so I have reopened the issue again to investigate why this is happening.

ruudvanbuul commented 1 year ago

@meeroslav I tried using the workspaces fix you mentioned above, but it doesn't install the packages, neither in the root node_modules nor in the lib node_modules.

  1. add workspaces to root package.json
  2. rm -rf node_modules (libs)
  3. remove package-lock.json from libs
  4. rm -rf node_modules (root level)
  5. yarn --frozen-lockfile

Am I missing something?

meeroslav commented 1 year ago

You can check the official yarn documentation here: https://classic.yarnpkg.com/lang/en/docs/workspaces/

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.