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') #2388

Closed
tomdglenn91 opened this issue Feb 21, 2022 · 8 comments

Comments

@tomdglenn91
Copy link

So this one may be difficult to reproduce as I cannot replicate in a sandbox, but in my big codebase it breaks.

I have a mixture of javascript and typescript as we migrate over to ts, and have just switched on this rule. I have it configured as such:

'import/no-unused-modules': [
          'warn',
          {
            unusedExports: true,
            src: [path.join(__dirname, 'src', 'main')],
            ignoreExports: ['**/white/**'],
          },
        ],

This is turned on for both javascript and the typescript overrides. I basically do not want to use this rule in anything that exists in a folder called white. This is due to some module hacking we are doing to overwrite code at build time.

I get the following stack trace when it errors:

❯ yarn run eslint --ext .js --ext .jsx --ext .ts --ext .tsx src/main/constants
yarn run v1.22.17
$ /Users/tom/code/projects/frontend/node_modules/.bin/eslint --ext .js --ext .jsx --ext .ts --ext .tsx src/main/constants

Oops! Something went wrong! :(

ESLint: 7.32.0

TypeError: Error while loading rule 'import/no-unused-modules': Cannot read properties of undefined (reading 'get')
Occurred while linting /Users/tom/code/projects/frontend/src/main/constants/access-permissions.js
    at /Users/tom/code/projects/frontend/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:277:44
    at Set.forEach (<anonymous>)
    at /Users/tom/code/projects/frontend/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:274:11
    at Map.forEach (<anonymous>)
    at prepareImportsAndExports (/Users/tom/code/projects/frontend/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:273:13)
    at doPreparation (/Users/tom/code/projects/frontend/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:344:3)
    at Object.create (/Users/tom/code/projects/frontend/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js:484:9)
    at createRuleListeners (/Users/tom/code/projects/frontend/node_modules/eslint/lib/linter/linter.js:765:21)
    at /Users/tom/code/projects/frontend/node_modules/eslint/lib/linter/linter.js:937:31
    at Array.forEach (<anonymous>)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I can get around this by going into the code itself in node_modules and changing:

var currentExports = exportList.get(val);
var currentExport = currentExports.get(EXPORT_ALL_DECLARATION);
currentExport.whereUsed.add(key);

to:

var currentExports = exportList.get(val);
if (currentExports) {
  var currentExport = currentExports.get(EXPORT_ALL_DECLARATION);
  currentExport.whereUsed.add(key);
}

I can raise a PR for this if necessary, however before doing so I wanted to check if this seems like a valid solution as I wasn't sure if it could be something my side (I also couldn't figure out your testing pattern in the 10 minutes i looked!)

Thanks.

@ljharb
Copy link
Member

ljharb commented Feb 21, 2022

This plugin will crawl your entire dep graph, including stuff in the white folder. You may need the import/ignore setting.

Can you share the code that it’s erroring on?

@tomdglenn91
Copy link
Author

The file it falls over on is a simple constants file.

export const REGISTER = 'register'
export const CLASS = 's_class'
export const ORDERS = 's_orders'

For experimentation, I tried just deleting this file and it then errors on the next file down the list. I assume it's just the first file it tries to lint.

I can try adding the same glob to import/ignore, but to be honest I didn't quite understand the import/ignore rule to begin with 😞 .

@ljharb
Copy link
Member

ljharb commented Feb 21, 2022

In this case, I’m talking about a global setting, not a rule - see the docs.

I assume even though you’re on eslint 7, you’re on the latest version of the plugin?

what file imports that access-permissions file?

@tomdglenn91
Copy link
Author

tomdglenn91 commented Feb 21, 2022

Ah sorry I meant the global setting yeah.

I am indeed on the latest version (^2.25.4) with eslint 7. Have tried upgrading to eslint 8 but the issue still occurs.

It's imported all over the place which may make this more difficult. An example is we have it imported from constants/permissions, then from helpers. It's a pretty big spaghetti mess.

I went to the extreme and deleted all of constants as well as some other huge directories and the issue still occurs.

edit: I deleted basically everything and started adding it back in slowly to find a culprit. It started erroring the second I added a folder back in that contained a white folder.

I should have mentioned this fact earlier (sorry) as it may be important: Files inside the white folder are never imported directly. Would this be a candidate to add it to the import/ignore config?

@tomdglenn91
Copy link
Author

I've managed to recreate it! The culprit seems to be when there's an index file. I don't know the specifics without digging into your code (stuck at work right now 😭 ) .

I've managed however to recreate it in a tiny little repository. I hope this format is okay to observe the issue:

Screenshot 2022-02-21 at 18 57 37

The second i delete the index.js file, everything is all good.

Screenshot 2022-02-21 at 18 58 14

@ljharb
Copy link
Member

ljharb commented Mar 1, 2022

@tomdglenn91 could you share that repo? That would make it much easier to fix this :-)

@tomdglenn91
Copy link
Author

@tomdglenn91 could you share that repo? That would make it much easier to fix this :-)

Hey sorry for the radio silence. I've pushed the repo up here:
https://github.com/tomdglenn91/import-lint-issue

If you try to run the linter, you get an error , despite the folder being ignored. If you remove the index.js file however, the linter will run again fine. I've gotten around this by adding the matching ignoreExports to import/ignore config as you mentioned above.

ljharb added a commit to ljharb/eslint-plugin-import that referenced this issue Mar 13, 2022
ljharb added a commit to ljharb/eslint-plugin-import that referenced this issue Mar 13, 2022
@ljharb ljharb closed this as completed in 8b7000e Mar 13, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Apr 9, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) | devDependencies | minor | [`2.25.4` -> `2.26.0`](https://renovatebot.com/diffs/npm/eslint-plugin-import/2.25.4/2.26.0) |

---

### Release Notes

<details>
<summary>import-js/eslint-plugin-import</summary>

### [`v2.26.0`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#&#8203;2260---2022-04-05)

[Compare Source](import-js/eslint-plugin-import@v2.25.4...v2.26.0)

##### Added

-   \[`no-named-default`, `no-default-export`, `prefer-default-export`, `no-named-export`, `export`, `named`, `namespace`, `no-unused-modules`]: support arbitrary module namespace names (\[[#&#8203;2358](import-js/eslint-plugin-import#2358)], thanks \[[@&#8203;sosukesuzuki](https://github.com/sosukesuzuki)])
-   \[`no-dynamic-require`]: support dynamic import with espree (\[[#&#8203;2371](import-js/eslint-plugin-import#2371)], thanks \[[@&#8203;sosukesuzuki](https://github.com/sosukesuzuki)])
-   \[`no-relative-packages`]: add fixer (\[[#&#8203;2381](import-js/eslint-plugin-import#2381)], thanks \[[@&#8203;forivall](https://github.com/forivall)])

##### Fixed

-   \[`default`]: `typescript-eslint-parser`: avoid a crash on exporting as namespace (thanks \[[@&#8203;ljharb](https://github.com/ljharb)])
-   \[`export`]/TypeScript: false positive for typescript namespace merging (\[[#&#8203;1964](import-js/eslint-plugin-import#1964)], thanks \[[@&#8203;magarcia](https://github.com/magarcia)])
-   \[`no-duplicates`]: ignore duplicate modules in different TypeScript module declarations (\[[#&#8203;2378](import-js/eslint-plugin-import#2378)], thanks \[[@&#8203;remcohaszing](https://github.com/remcohaszing)])
-   \[`no-unused-modules`]: avoid a crash when processing re-exports (\[[#&#8203;2388](import-js/eslint-plugin-import#2388)], thanks \[[@&#8203;ljharb](https://github.com/ljharb)])

##### Changed

-   \[Tests] `no-nodejs-modules`: add tests for node protocol URL (\[[#&#8203;2367](import-js/eslint-plugin-import#2367)], thanks \[[@&#8203;sosukesuzuki](https://github.com/sosukesuzuki)])
-   \[Tests] `default`, `no-anonymous-default-export`, `no-mutable-exports`, `no-named-as-default-member`, `no-named-as-default`: add tests for arbitrary module namespace names (\[[#&#8203;2358](import-js/eslint-plugin-import#2358)], thanks \[[@&#8203;sosukesuzuki](https://github.com/sosukesuzuki)])
-   \[Docs] \[`no-unresolved`]: Fix RegExp escaping in readme (\[[#&#8203;2332](import-js/eslint-plugin-import#2332)], thanks \[[@&#8203;stephtr](https://github.com/stephtr)])
-   \[Refactor] `namespace`: try to improve performance (\[[#&#8203;2340](import-js/eslint-plugin-import#2340)], thanks \[[@&#8203;ljharb](https://github.com/ljharb)])
-   \[Docs] make rule doc titles consistent (\[[#&#8203;2393](import-js/eslint-plugin-import#2393)], thanks \[[@&#8203;TheJaredWilcurt](https://github.com/TheJaredWilcurt)])
-   \[Docs] `order`: TS code examples should use TS code blocks (\[[#&#8203;2411](import-js/eslint-plugin-import#2411)], thanks \[[@&#8203;MM25Zamanian](https://github.com/MM25Zamanian)])
-   \[Docs] `no-unresolved`: fix link (\[[#&#8203;2417](import-js/eslint-plugin-import#2417)], thanks \[[@&#8203;kylemh](https://github.com/kylemh)])

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1284
Reviewed-by: crapStone <crapstone@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@doctorby
Copy link

The issue is still there in 2.27.5 version

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

3 participants