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

[Fix] no-extraneous-dependencies: allow incorrect paths in packageDir #3012

Merged

Conversation

chabb
Copy link

@chabb chabb commented Jun 3, 2024

This rule can potentially fails ( = reports incorrect errors ) in a monorepo settings; this is due to the fact that the building of the map of the available installed dependencies stops when there is an error ( e.g when the path to the package.json does not point to an existing package.json file )

  • If you pass only one path to a package.json file, then this path should be correct, this is ok
  • If you pass multiple paths, there are some situations when those paths point to a wrong path, this happens typically in a nx monorepo. The problem is that the parsing of the package.json files stop is one of the path is wrong, and then the rule will give us false positive. One example is a nx monorepo with a pre-commit hook that runs eslint
    -- NX will run eslint in the projects folder, so we need to grab the root package.json
    -- The pre-commit hook will run eslint in the root folder, so one of the path given will be an incorrect path, but we do not want to throw there, otherwise the rull will fail

- If you pass only one path to a package.json file, then this path
should be correct
- If you pass multiple paths, there are some situations when those paths
point to a wrong path, this happens typically in a nx monorepo with husky
-- NX will run eslint in the projects folder, so we need to grab the
root package.json
-- Husky will run in the root folder, so one of the path given will be
an incorrect path, but we do not want throw there, otherwise the rull
will fail
@chabb chabb changed the title [fix] no-extraneous-dependencies allow wrong path [fix] no-extraneous-dependencies allow incorrect paths in packageDir Jun 3, 2024
@chabb chabb force-pushed the fix/allow-incorrect-path-if-mulitple-paths branch from 11aebc1 to f0c9fd2 Compare June 3, 2024 11:57
Copy link

codecov bot commented Jun 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.90%. Comparing base (6554bd5) to head (f0c9fd2).

Current head f0c9fd2 differs from pull request most recent head fc361a9

Please upload reports for the commit fc361a9 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3012      +/-   ##
==========================================
+ Coverage   95.87%   95.90%   +0.03%     
==========================================
  Files          78       78              
  Lines        3293     3296       +3     
  Branches     1156     1158       +2     
==========================================
+ Hits         3157     3161       +4     
+ Misses        136      135       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danymarques
Copy link

Would be nice to have this merged! We also have issues with this.

@alaingiller
Copy link

Same here, this rule doesn't work in a nx monorepo setup

@hedinfaok
Copy link

As a workaround, I run nx run-many -t lint in the repo root directory. The eslint config file is also in the repo root directory.

    'import/no-extraneous-dependencies': [
      'warn',
      {
        packageDir: [
          __dirname, // <root>/package.json
          process.cwd(), // <project>/package.json
        ],
      },
    ],

@ljharb ljharb force-pushed the fix/allow-incorrect-path-if-mulitple-paths branch from f0c9fd2 to fc361a9 Compare June 25, 2024 06:30
@ljharb ljharb changed the title [fix] no-extraneous-dependencies allow incorrect paths in packageDir [Fix] no-extraneous-dependencies: allow incorrect paths in packageDir Jun 25, 2024
@ljharb ljharb merged commit fc361a9 into import-js:main Jun 25, 2024
309 checks passed
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Sep 4, 2024
| datasource | package              | from   | to     |
| ---------- | -------------------- | ------ | ------ |
| npm        | eslint-plugin-import | 2.29.1 | 2.30.0 |


## [v2.30.0](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#2300---2024-09-02)

##### Added

-   \[`dynamic-import-chunkname`]: add `allowEmpty` option to allow empty leading comments (\[[#2942](import-js/eslint-plugin-import#2942)], thanks \[[@JiangWeixian](https://github.com/JiangWeixian)])
-   \[`dynamic-import-chunkname`]: Allow empty chunk name when webpackMode: 'eager' is set; add suggestions to remove name in eager mode (\[[#3004](import-js/eslint-plugin-import#3004)], thanks \[[@amsardesai](https://github.com/amsardesai)])
-   \[`no-unused-modules`]: Add `ignoreUnusedTypeExports` option (\[[#3011](import-js/eslint-plugin-import#3011)], thanks \[[@silverwind](https://github.com/silverwind)])
-   add support for Flat Config (\[[#3018](import-js/eslint-plugin-import#3018)], thanks \[[@michaelfaith](https://github.com/michaelfaith)])

##### Fixed

-   \[`no-extraneous-dependencies`]: allow wrong path (\[[#3012](import-js/eslint-plugin-import#3012)], thanks \[[@chabb](https://github.com/chabb)])
-   \[`no-cycle`]: use scc algorithm to optimize (\[[#2998](import-js/eslint-plugin-import#2998)], thanks \[[@soryy708](https://github.com/soryy708)])
-   \[`no-duplicates`]: Removing duplicates breaks in TypeScript (\[[#3033](import-js/eslint-plugin-import#3033)], thanks \[[@yesl-kim](https://github.com/yesl-kim)])
-   \[`newline-after-import`]: fix considerComments option when require (\[[#2952](import-js/eslint-plugin-import#2952)], thanks \[[@developer-bandi](https://github.com/developer-bandi)])
-   \[`order`]: do not compare first path segment for relative paths (\[[#2682](import-js/eslint-plugin-import#2682)]) (\[[#2885](import-js/eslint-plugin-import#2885)], thanks \[[@mihkeleidast](https://github.com/mihkeleidast)])

##### Changed

-   \[Docs] `no-extraneous-dependencies`: Make glob pattern description more explicit (\[[#2944](import-js/eslint-plugin-import#2944)], thanks \[[@mulztob](https://github.com/mulztob)])
-   \[`no-unused-modules`]: add console message to help debug \[[#2866](import-js/eslint-plugin-import#2866)]
-   \[Refactor] `ExportMap`: make procedures static instead of monkeypatching exportmap (\[[#2982](import-js/eslint-plugin-import#2982)], thanks \[[@soryy708](https://github.com/soryy708)])
-   \[Refactor] `ExportMap`: separate ExportMap instance from its builder logic (\[[#2985](import-js/eslint-plugin-import#2985)], thanks \[[@soryy708](https://github.com/soryy708)])
-   \[Docs] `order`: Add a quick note on how unbound imports and --fix (\[[#2640](import-js/eslint-plugin-import#2640)], thanks \[[@MinervaBot](https://github.com/minervabot)])
-   \[Tests] appveyor -> GHA (run tests on Windows in both pwsh and WSL + Ubuntu) (\[[#2987](import-js/eslint-plugin-import#2987)], thanks \[[@joeyguerra](https://github.com/joeyguerra)])
-   \[actions] migrate OSX tests to GHA (\[[ljharb#37](ljharb/eslint-plugin-import#37)], thanks \[[@aks-](https://github.com/aks-)])
-   \[Refactor] `exportMapBuilder`: avoid hoisting (\[[#2989](import-js/eslint-plugin-import#2989)], thanks \[[@soryy708](https://github.com/soryy708)])
-   \[Refactor] `ExportMap`: extract "builder" logic to separate files (\[[#2991](import-js/eslint-plugin-import#2991)], thanks \[[@soryy708](https://github.com/soryy708)])
-   \[Docs] \[`order`]: update the description of the `pathGroupsExcludedImportTypes` option (\[[#3036](import-js/eslint-plugin-import#3036)], thanks \[[@liby](https://github.com/liby)])
-   \[readme] Clarify how to install the plugin (\[[#2993](import-js/eslint-plugin-import#2993)], thanks \[[@jwbth](https://github.com/jwbth)])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants