import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.55k stars 1.56k forks source link

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

Open magic-m-johnson opened 1 year ago

magic-m-johnson commented 1 year ago

was closed here, but is still randomly occurring today:

https://github.com/import-js/eslint-plugin-import/issues/2388

ljharb commented 1 year ago

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

iz-podpolja commented 1 year ago

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 commented 1 year ago

What's LSP?

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

ljharb commented 1 year ago

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

iz-podpolja commented 1 year ago

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 commented 1 year ago

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

iz-podpolja commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 9 months ago

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 commented 9 months ago

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

$ git clone git@github.com: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 ```txt [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 () 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 commented 9 months ago

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

olsonpm commented 9 months ago

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

olsonpm commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

headfire94 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

@WavyWalk https://github.com/import-js/eslint-plugin-import/issues/2866#issuecomment-1719863167

rishitank commented 3 weeks ago

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

headfire94 commented 3 weeks ago

@rishitank fix was here https://github.com/import-js/eslint-plugin-import/pull/2990, but team don't want to merge it without unit test. i suggest to fork

ljharb commented 3 weeks ago

@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.