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

Lint breaks after updating from 19.0.5 to 19.0.6 #25319

Closed
1 of 4 tasks
TomTomB opened this issue May 23, 2024 · 14 comments · Fixed by #25938
Closed
1 of 4 tasks

Lint breaks after updating from 19.0.5 to 19.0.6 #25319

TomTomB opened this issue May 23, 2024 · 14 comments · Fixed by #25938
Assignees
Labels
scope: linter Issues related to Eslint support in Nx type: bug

Comments

@TomTomB
Copy link

TomTomB commented May 23, 2024

Current Behavior

After updating to nx 19.0.6, linting will fail inside an angular monorepo. This only ocurres if the user has updated the @typescript-eslint/* packages from 7.3.0 to e.g. latest (7.10.0). I guess this has to do with #24632.

Expected Behavior

Linting should not break during patch releases.

GitHub Repo

No response

Steps to Reproduce

  1. Create a default nx monorepo with the angular preset and choose integrated instead of standalone.
  2. Update @typescript-eslint/eslint-plugin and @typescript-eslint/parser to 7.10.0
  3. Lint. npx nx run-many -t lint --skip-nx-cache

Nx Report

Node   : 20.11.1
OS     : linux-x64 (WSL)
yarn   : 1.22.21

nx                 : 19.0.6
@nx/js             : 19.0.6
@nx/jest           : 19.0.6
@nx/linter         : 19.0.6
@nx/eslint         : 19.0.6
@nx/workspace      : 19.0.6
@nx/angular        : 19.0.6
@nx/devkit         : 19.0.6
@nx/eslint-plugin  : 19.0.6
@nx/playwright     : 19.0.6
@nrwl/tao          : 19.0.6
@nx/web            : 19.0.6
@nx/webpack        : 19.0.6
typescript         : 5.4.5
---------------------------------------
Registered Plugins:
@nx/playwright/plugin
@nx/eslint/plugin

Failure Logs

✖  nx run eslint-issue:lint
      Linting "eslint-issue"...
      
       NX   Error while loading rule '@angular-eslint/template/banana-in-box': You have used a rule which requires '@angular-eslint/template-parser' to be used as the 'parser' in your ESLint config.
      
      Occurred while linting some/file/path....

Package Manager Version

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

No response

@mgansler
Copy link
Contributor

mgansler commented May 23, 2024

I have noticed a very similar behaviour:

Linting "components"...
 NX   context.getScope is not a function
Occurred while linting /builds/mgansler/plusone/libs/components/cypress/support/commands.ts:14
Rule: "import/no-amd"

Worked fine with 19.0.5 but is broken with 19.0.6 and no other changes.
The version of @typescript-eslint/* seems to make no difference for me, same result with 7.3.0 and 7.10.0

@SteveW94
Copy link

I experienced the same.

Due to this the Update PR is currently on hold

@Tijawk
Copy link

Tijawk commented May 23, 2024

I have huge performance issue with lerna since I have updated nx from 19.0.5 to 19.0.6. I use @typescript-eslint/*@7.10.0 too,

I think it's the same probleme. I have rollback to 19.0.5.

@FrozenPandaz
Copy link
Collaborator

I have huge performance issue with lerna since I have updated nx from 19.0.5 to 19.0.6. I use @typescript-eslint/*@7.10.0 too,

I think it's the same probleme. I have rollback to 19.0.5.

Mm.. this isn't much details but I think this might be a separate issue unrelated to this one.

NX context.getScope is not a function

Thank you for reporting it. We'll look into it.

@FrozenPandaz FrozenPandaz added the scope: linter Issues related to Eslint support in Nx label May 23, 2024
@JamesHenry
Copy link
Collaborator

@TomTomB @mgansler @SteveW94 Maybe you are now pulling in eslint v9?

Please try running this command or similar and see what version of eslint is getting pulled in:

npm ls eslint

@JamesHenry
Copy link
Collaborator

@Tijawk Please open a fresh issue for your case with maximum information/steps to reproduce

@TomTomB
Copy link
Author

TomTomB commented May 23, 2024

@JamesHenry Both eslint versions (8.57.x, and 9.x) result in the same error.

@SteveW94
Copy link

@JamesHenry
This is before the installation of 19.0.6

image

As you can see no eslint 9
Just after installation of 19.0.6

image

You can see, @nx/eslint@19.0.6 is the culprit, as it references eslint 9!

In my package .json there is:
"eslint": "8.57.0" pinned. So no Semver Tag or anything.

So I would say, this way ESLint "sneaks" in there should not be intended!

@mgansler
Copy link
Contributor

with 19.0.5:

yarn why v1.22.22
[1/4] 🤔  Why do we have the module "eslint"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "eslint@8.57.0"
info Has been hoisted to "eslint"
info Reasons this module exists
   - Specified in "devDependencies"
   - Hoisted from "@nx#eslint#eslint"
info Disk size without dependencies: "5.5MB"
info Disk size with unique dependencies: "15.11MB"
info Disk size with transitive dependencies: "18.44MB"
info Number of shared dependencies: 80

With 19.0.6:

yarn why v1.22.22
[1/4] 🤔  Why do we have the module "eslint"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "eslint@8.57.0"
info Has been hoisted to "eslint"
info This module exists because it's specified in "devDependencies".
info Disk size without dependencies: "5.38MB"
info Disk size with unique dependencies: "14.99MB"
info Disk size with transitive dependencies: "18.29MB"
info Number of shared dependencies: 82
=> Found "@nx/eslint#eslint@9.3.0"
info This module exists because "@nx#eslint" depends on it.
info Disk size without dependencies: "3.8MB"
info Disk size with unique dependencies: "11.71MB"
info Disk size with transitive dependencies: "15.73MB"
info Number of shared dependencies: 80

And with eslint removed from my package.json (and Nx 19.0.6):

y why eslint
yarn why v1.22.22
[1/4] 🤔  Why do we have the module "eslint"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "eslint@9.3.0"
info Reasons this module exists
   - "@nx#eslint" depends on it
   - Hoisted from "@nx#eslint#eslint"
info Disk size without dependencies: "5.34MB"
info Disk size with unique dependencies: "14.5MB"
info Disk size with transitive dependencies: "18.33MB"
info Number of shared dependencies: 71

Same error though, still context.getScope is not a function

@mgansler
Copy link
Contributor

mgansler commented May 23, 2024

When I force eslint to 8.57.0, linting works:

 "resolutions": {
    "eslint": "8.57.0"
  }
yarn why v1.22.22
[1/4] 🤔  Why do we have the module "eslint"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "eslint@8.57.0"
info Has been hoisted to "eslint"
info Reasons this module exists
   - Specified in "devDependencies"
   - Hoisted from "@nx#eslint#eslint"
info Disk size without dependencies: "5.5MB"
info Disk size with unique dependencies: "15.11MB"
info Disk size with transitive dependencies: "18.44MB"
info Number of shared dependencies: 80

Can someone with npm try this with overrides instead of resolutions?

FrozenPandaz added a commit that referenced this issue May 23, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
<!-- This is the behavior we have today -->

`@nx/eslint` is bringing in `eslint v9` prematurely.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

`@nx/eslint` should not bring in `eslint v9` yet. Though, it does have
support for having `v9` installed at the workspace level.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #25319
@SteveW94
Copy link

👍🏼

FrozenPandaz added a commit that referenced this issue May 23, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
<!-- This is the behavior we have today -->

`@nx/eslint` is bringing in `eslint v9` prematurely.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

`@nx/eslint` should not bring in `eslint v9` yet. Though, it does have
support for having `v9` installed at the workspace level.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #25319

(cherry picked from commit f728058)
FrozenPandaz added a commit that referenced this issue May 23, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
<!-- This is the behavior we have today -->

`@nx/eslint` is bringing in `eslint v9` prematurely.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

`@nx/eslint` should not bring in `eslint v9` yet. Though, it does have
support for having `v9` installed at the workspace level.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #25319

(cherry picked from commit f728058)
@JamesHenry
Copy link
Collaborator

JamesHenry commented May 24, 2024

FYI we confirmed this is only an issue with yarn. The other package managers behave how we expected and do not force the eslint v9 dependency upon you. We are still discussing the ultimate resolution to this.

@terrymun
Copy link

Can confirm that this is also an issue with pnpm too. Currently on nx@19.2.3 and upgrading eslint from 8.57.0 → 9.5.0 is causing issues.

@JamesHenry
Copy link
Collaborator

@terrymun It's better to open a fresh issue with full context about your specific circumstances, rather than commenting on closed issues. If you are choosing to upgrade from 8 to 9 as your comment suggests then it already confirms it is not directly related to this specific issue.

If you open a fresh issue the team will happily take a look

@nrwl nrwl locked as resolved and limited conversation to collaborators Jun 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
scope: linter Issues related to Eslint support in Nx type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants