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

Error while loading rule 'import/no-unused-modules': Cannot read properties of undefined (reading 'get') #2866

Open
magic-m-johnson opened this issue Aug 28, 2023 · 23 comments

Comments

@magic-m-johnson
Copy link

was closed here, but is still randomly occurring today:

#2388

@ljharb
Copy link
Member

ljharb commented Aug 30, 2023

Can you provide more info? The code being linted, or a repro repo?

@iz-podpolja
Copy link

This occurs for us only when run through LSP, I cannot provide a full repro, but the following patch fixes it locally:

diff --git a/lib/rules/no-unused-modules.js b/lib/rules/no-unused-modules.js
index 098f72a5340a706e0652a272e96d473b6abc6051..d248367f5c4ab68d69806a762b8a90cdeb11e43b 100644
--- a/lib/rules/no-unused-modules.js
+++ b/lib/rules/no-unused-modules.js
@@ -525,7 +525,7 @@ module.exports = {
           exports = exportList.get(file);
 
           // special case: export * from
-          var exportAll = exports.get(EXPORT_ALL_DECLARATION);
+          var exportAll = exports && exports.get(EXPORT_ALL_DECLARATION);
           if (typeof exportAll !== 'undefined' && exportedValue !== IMPORT_DEFAULT_SPECIFIER) {
             if (exportAll.whereUsed.size > 0) {
               return;
@@ -533,7 +533,7 @@ module.exports = {
           }
 
           // special case: namespace import
-          var namespaceImports = exports.get(IMPORT_NAMESPACE_SPECIFIER);
+          var namespaceImports = exports && exports.get(IMPORT_NAMESPACE_SPECIFIER);
           if (typeof namespaceImports !== 'undefined') {
             if (namespaceImports.whereUsed.size > 0) {
               return;
@@ -543,7 +543,7 @@ module.exports = {
           // exportsList will always map any imported value of 'default' to 'ImportDefaultSpecifier'
           var exportsKey = exportedValue === DEFAULT ? IMPORT_DEFAULT_SPECIFIER : exportedValue;
 
-          var exportStatement = exports.get(exportsKey);
+          var exportStatement = exports && exports.get(exportsKey);
 
           var value = exportsKey === IMPORT_DEFAULT_SPECIFIER ? DEFAULT : exportsKey;

@ljharb
Copy link
Member

ljharb commented Sep 14, 2023

What's LSP?

Thanks, that helps; I'll take a look (altho I'm still confused why exports there would be falsy)

@ljharb
Copy link
Member

ljharb commented Sep 14, 2023

In order to fix this, I need a regression test. Are you sure you can't provide some code that triggers this?

@iz-podpolja
Copy link

iz-podpolja commented Sep 15, 2023

What's LSP?

Thanks, that helps; I'll take a look (altho I'm still confused why exports there would be falsey)

LSP -> Language Server Protocol, in my particular case, eslint was run by a VSCode extension.

In order to fix this, I need a regression test. Are you sure you can't provide some code that triggers this?

Yeah unfortunately it's a big private monorepo, and my attempts to distill the issue were in vain

@ljharb
Copy link
Member

ljharb commented Sep 15, 2023

Is this an issue with code in the middle of typing? or is it with code that parses correctly?

@iz-podpolja
Copy link

At least in my case, it's enough to open the file to cause eslint service in the IDE to crash in these specific places

@ljharb
Copy link
Member

ljharb commented Sep 15, 2023

Gotcha - and there's no way to share even the code in just that file? You're welcome to send it to me privately, or let me see it over screen share, if you prefer not to post it publicly.

@iz-podpolja
Copy link

@ljharb It's not one file though - it's actually any file triggering eslint in the whole monorepo. As for the sharing session - I'd need to ask - however I'm pretty sure I could send you at the very least some screencasts of the issue and relevant config files. Perhaps Thursday~ish?

@ljharb
Copy link
Member

ljharb commented Sep 18, 2023

Sounds good. If it’s really any file - and assuming there’s not something consistent across every file (like, every file uses jsx, or every file is native ESM, or something) then that suggests it’s part of the eslint config, which may include a custom parser.

Could you provide your eslint config?

@olsonpm
Copy link

olsonpm commented Jan 20, 2024

I'm finally digging into this now. In my case it's a vscode error that prompts upon copy/pasting a file. Reloading the window resolves it but hopefully I can get a PR or at least a failing test today.

@olsonpm
Copy link

olsonpm commented Jan 20, 2024

Okay I've distilled the issue at least from how I encounter it

$ git clone [email protected]:olsonpm/repro.git
$ cd repro
$ git checkout eslint-plugin-import-issue-2866
$ npm ci
$ code .
# then when in vscode, select the file `dep.mjs` and copy/paste it to create `dep copy.mjs`
# at this point you'll trigger the error
Error in vscode
[Error - 10:55:46 AM] An unexpected error occurred:
[Error - 10:55:46 AM] TypeError: Cannot read properties of undefined (reading 'get')
Occurred while linting /path/to/repro/dep copy.mjs:1
Rule: "import/no-unused-modules"
    at checkUsage (/path/to/repro/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:533:35)
    at ExportDefaultDeclaration (/path/to/repro/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:933:13)
    at ruleErrorHandler (/path/to/repro/node_modules/eslint/lib/linter/linter.js:1076:28)
    at /path/to/repro/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/path/to/repro/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/path/to/repro/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/path/to/repro/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/path/to/repro/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (/path/to/repro/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:803:23)

reproduce-error


I looked into creating a failing test but it's simply the file not existing exportList at the point it's traversed - i.e. Program:exit hasn't been called yet.

LJharb - if you have a suggestion on how you'd want a fix implemented then I'm willing to create a PR.

@ljharb
Copy link
Member

ljharb commented Jan 20, 2024

hmm, that's not reproducing the error for me. what version of vscode do you have? I have 1.85.1.

@olsonpm
Copy link

olsonpm commented Jan 21, 2024

yeah 1.85.1 on my end too. Tomorrow I'll get this in a clean ubuntu vm to rule out settings/plugins.

@olsonpm
Copy link

olsonpm commented Jan 24, 2024

Okay I was able to reproduce in a fresh ubuntu instance. Not sure if you feel like messing with an image but here's a link with everything ready. After importing the image into virtualbox, I reproduced the error via

# close initial "Online Accounts" window and open terminal
$ cd git-repos/personal/repro/
$ rm dep\ copy.mjs
$ code .
# default keyring nonsense pops up.  pass = "testing"
# close the "dep\ copy.mjs" tab
# ctrl+shift+p -> reload window
# in left file panel, select 'dep.mjs' then ctrl+c -> ctrl+v
# error will show up in output

@OnkelTem
Copy link

I don't know if it's related but it's happening very often when I create some files in WebStorm. Since it doesn't re-run ESLint (they promised to implement it the next version if I'm not mistaking) I have to restart WebStorm every time it happens because it doesn't want to go away.

This is how it usually looks:

image

image

As you see there is nothing wrong with the code now. But maybe there was an error while I was editing it. Unfortunately I cannot track back to find out the reason.

@ljharb
Copy link
Member

ljharb commented Feb 14, 2024

The next release will have a console error with the filename right before this crash, to help debug it further.

@headfire94
Copy link

Unfortunately I cannot track back to find out the reason.

Same, This happens because variable in new file is not imported anywhere yet, once i import it linter resolves, but until then auto-fix functionality doesn't work

@WavyWalk
Copy link

We have same problems (different OS's), patching helps. To whoever maintains - looks like suggested fix (exports?.get) works and can be safely merged.

@ljharb
Copy link
Member

ljharb commented Mar 27, 2024

@WavyWalk #2866 (comment)

renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this issue 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)])
@rishitank
Copy link

Hi, is there a timeline on when this issue will be fixed? Thanks.

@headfire94
Copy link

headfire94 commented Oct 17, 2024

@rishitank fix was here #2990, but team don't want to merge it without unit test. i suggest to fork

@ljharb
Copy link
Member

ljharb commented Oct 17, 2024

@headfire94 i don't think forking is going to help people; if there's no test then you can't prove it works, and a fork with an untested change will thus be broken. Additionally, by you deleting the fork, that PR is unrecoverable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants