TypeError: Cannot read property 'lastIndexOf' of undefined after upgrading from 2.23.4 to 2.24.1

[mertia-himanshu]

Steps to Reproduce
npm install
npm run fix:eslint

Repo link - https://github.com/tmtsoftware/esw-ocs-eng-ui/tree/eslint-plugin-import-issue
Branch - eslint-plugin-import-issue

TypeError: Cannot read property 'lastIndexOf' of undefined
Occurred while linting /Users/himanshu/projects/tmt/esw-ocs-eng-ui/test/utils/test-utils.tsx:1
    at removeQuerystring (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-import-resolver-typescript/lib/cjs.js:124:31)
    at Object.resolve (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-import-resolver-typescript/lib/cjs.js:51:14)
    at v2 (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-module-utils/resolve.js:117:23)
    at withResolver (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-module-utils/resolve.js:122:14)
    at fullResolve (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-module-utils/resolve.js:139:22)
    at Function.relative (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-module-utils/resolve.js:84:10)
    at remotePath (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-plugin-import/lib/ExportMap.js:743:379)
    at resolveImport (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-plugin-import/lib/ExportMap.js:743:460)
    at Object.getImport (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-plugin-import/lib/ExportMap.js:745:107)
    at ExportMap.get (/Users/himanshu/projects/tmt/esw-ocs-eng-ui/node_modules/eslint-plugin-import/lib/ExportMap.js:710:667)
ERROR: "eslint:check" exited with 2.```

[mmakarin]

Same here

[ljharb]

That repo unfortunately doesn't reproduce the issue for me (with npm run eslint:check either).

@mmakarin do you have a repro repo?

[ljharb]

Also, your stack trace seems like the error is coming from eslint-import-resolver-typescript - which is a separate project from this one. Can you file it there?

I'll be happy to reopen this if I can reproduce it and if the error is coming from projects in this repo.

[jjangga0214]

@ljharb This happens to me as well.
Maybe I can provide a repro later, but I want you to know that this may not be misreported.

[ljharb]

@jjangga0214 i believe it’s happening, but if it’s an error in eslint-import-resolver-typescript, that’s not a package i maintain and isn’t part of this repo.

[NickHeiner]

I also got this, only after updating to 2.24.1. I'm trying to create a repro now.

[NickHeiner]

Partial info: This triggers on a line like:

source.ts:

import {a} from './b';

where b.ts is:

export * as b from './c';

And c.ts can be empty.

However, I recreated this precise setup in a standalone repo, and couldn't repro. So there's a necessary element I'm missing.

Separately, when inspecting the eslint-plugin-import source code, I found that if I insert a console.log in lib/ExportMap.js:

function processSpecifier(s, n, m) {var nsource = n.source && n.source.value; console.log({n}); 

I see the following output:

{
  n: {
    type: 'Identifier',
    name: 'clConstants',
    range: [ 647, 658 ],
    loc: { start: [Object], end: [Object] }
  }
}

And it does not appear that resolveImport, in the processSpecifier block, handles n.source being undefined.

[NickHeiner]

When I add that same console line in the repo where the issue doesn't repro, the line isn't hit at all. So it appears that processSpecifier isn't being called at all.

Does that help any figuring out what's going on? Ha.

[ljharb]

It might be the case that resolvers weren't previously passed undefined, and now we are doing that, so even though the error is coming from the typescript resolver (and even though they could handle it), we can probably fix the problem a bit differently.

I'll try to create a failing test case; if I can, then we can solve it (but either way, it would be ideal to file a bug on the typescript resolver's repo)

[NickHeiner]

Here's the output of yarn list --pattern eslint in a repo where I can repro this, and one where I can't:

Issue is present:

yarn list v1.22.10
├─ @typescript-eslint/eslint-plugin@4.25.0
├─ @typescript-eslint/experimental-utils@4.25.0
├─ @typescript-eslint/parser@4.25.0
├─ @typescript-eslint/scope-manager@4.25.0
│  ├─ @typescript-eslint/visitor-keys@4.25.0
│  └─ eslint-visitor-keys@2.0.0
├─ @typescript-eslint/types@4.25.0
├─ @typescript-eslint/typescript-estree@4.25.0
│  ├─ @typescript-eslint/visitor-keys@4.25.0
│  └─ eslint-visitor-keys@2.0.0
├─ @typescript-eslint/visitor-keys@4.14.2
│  ├─ @typescript-eslint/types@4.14.2
│  └─ eslint-visitor-keys@2.0.0
├─ eslint-config-prettier@4.0.0
├─ eslint-config-tvui@3.11.0
├─ eslint-import-resolver-node@0.3.6
├─ eslint-import-resolver-typescript@2.4.0
├─ eslint-module-utils@2.6.2
├─ eslint-plugin-alphabetize@1.1.1
├─ eslint-plugin-import@2.24.1
├─ eslint-plugin-jest@24.1.3
│  ├─ @typescript-eslint/experimental-utils@4.14.2
│  ├─ @typescript-eslint/scope-manager@4.14.2
│  ├─ @typescript-eslint/types@4.14.2
│  └─ @typescript-eslint/typescript-estree@4.14.2
├─ eslint-plugin-nodequark@0.0.0-chunter.1
├─ eslint-plugin-package-json@0.1.3
├─ eslint-plugin-prettier@2.6.2
├─ eslint-plugin-react-hooks@4.2.0
├─ eslint-plugin-react@7.18.0
├─ eslint-plugin-tvui@3.10.0
│  └─ eslint-utils@1.3.1
├─ eslint-plugin-validate-markdown@0.0.0
├─ eslint-scope@5.0.0
├─ eslint-utils@2.0.0
├─ eslint-visitor-keys@1.1.0
├─ eslint@6.8.0
│  └─ eslint-utils@1.4.3
├─ jest-runner-eslint@0.10.0
└─ webpack@4.46.0
   └─ eslint-scope@4.0.3

Issue is not present:

yarn list v1.22.5
├─ @typescript-eslint/parser@4.25.0
├─ @typescript-eslint/scope-manager@4.25.0
├─ @typescript-eslint/types@4.25.0
├─ @typescript-eslint/typescript-estree@4.25.0
├─ @typescript-eslint/visitor-keys@4.14.2
│  └─ @typescript-eslint/types@4.14.2
├─ eslint-import-resolver-node@0.3.6
├─ eslint-import-resolver-typescript@2.4.0
├─ eslint-module-utils@2.6.2
├─ eslint-plugin-import@2.24.1
├─ eslint-scope@5.0.0
├─ eslint-utils@2.0.0
├─ eslint-visitor-keys@1.3.0
└─ eslint@6.8.0
Done in 0.17s.

Here's my unsuccessful attempt to repro this in a separate repo.

[ljharb]

ah - looks like we have a bunch of test failures (24) when I install eslint-import-resolver-typescript v2 locally - we only test with v1 atm. None of those failures, however, are this crash :-/ (many are `parseForESLint` from parser `@typescript-eslint/parser` is invalid and will just be ignored) We're also testing with v3 of @typescript-eslint/parser and apparently v4 is out. I filed #2203 to track that.

In the meantime, I could certainly avoid calling resolveImport with an undefined source, but without a regression test, that seems unwise :-/

@NickHeiner can you confirm that if you edit those two lines doing resolveImport(nsource) to resolveImport(nsource || s.source), it fixes the problem?

[NickHeiner]

That doesn't quite work. s is, for example:

{
    type: 'ExportAllDeclaration',
    source: {
      type: 'Literal',
      value: './constants',
      raw: "'./constants'",
      range: [Array],
      loc: [Object]
    },
    exportKind: 'value',
    exported: {
      type: 'Identifier',
      name: 'clConstants',
      range: [Array],
      loc: [Object]
    },
    range: [ 635, 678 ],
    loc: { start: [Object], end: [Object] }
  }

Whereas I think resolveImport expects a string. This results in stack trace:

TypeError: id.lastIndexOf is not a function
Occurred while linting /Users/nheiner/code/tvui/packages/darwin/src/components/PlaymodeLanguageDiscoveryControls/logger.ts:5
    at removeQuerystring (/Users/nheiner/code/tvui/node_modules/eslint-import-resolver-typescript/lib/cjs.js:125:31)
    at Object.resolve (/Users/nheiner/code/tvui/node_modules/eslint-import-resolver-typescript/lib/cjs.js:52:14)
    at v2 (/Users/nheiner/code/tvui/node_modules/eslint-module-utils/resolve.js:119:23)
    at withResolver (/Users/nheiner/code/tvui/node_modules/eslint-module-utils/resolve.js:124:14)
    at fullResolve (/Users/nheiner/code/tvui/node_modules/eslint-module-utils/resolve.js:141:22)
    at Function.relative (/Users/nheiner/code/tvui/node_modules/eslint-module-utils/resolve.js:85:10)
    at remotePath (/Users/nheiner/code/tvui/node_modules/eslint-plugin-import/lib/ExportMap.js:741:379)
    at resolveImport (/Users/nheiner/code/tvui/node_modules/eslint-plugin-import/lib/ExportMap.js:741:460)
    at Object.getImport (/Users/nheiner/code/tvui/node_modules/eslint-plugin-import/lib/ExportMap.js:743:134)
    at ExportMap.get (/Users/nheiner/code/tvui/node_modules/eslint-plugin-import/lib/ExportMap.js:708:667)

If I do s.source.value, then there is no crash. However, if I then change the import to import an invalid name, I don't get a lint error.

import {doesNotExist} from './b';