Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

import-name 6.2.0 ignoreExternalModules non-external modules that aren't TS or JS crash #883

Closed
dosentmatter opened this issue Jul 29, 2019 · 3 comments

Comments

@dosentmatter
Copy link

dosentmatter commented Jul 29, 2019

Bug Report

  • tslint-microsoft-contrib version: 6.2.0
  • TSLint version: 5.18.0
  • TypeScript version: 3.5.3
  • Running TSLint via: CLI

TypeScript code being linted

All the local files imported can just be empty.

import React from 'react'; // Doesn't cause an error
import blahSVG from './blah.svg'; // Errors
import fooSvg from './foo.svg'; // Doesn't cause an error. It doesn't go into checkIgnoreExternalModule() for some reason
import { blah } from './blah'; // blah.ts file

with tslint.json configuration:

{
  "rulesDirectory": ["tslint-microsoft-contrib"],
  "rules": {
    "import-name": true
  }
}

tsconfig.json. It needs to be passed to tslint -p, but an empty one is fine.

{
}

Actual behavior

I get this error

$ npx tslint -p tsconfig.json test.ts
The 'import-name' rule threw an error in '/Users/kevinlau/workspace/test/tslint-microsoft-contrib/test.ts':
TypeError: Cannot read property 'isExternalLibraryImport' of undefined
    at /Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:127:49
    at Map.forEach (<anonymous>)
    at checkIgnoreExternalModule (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:126:48)
    at isImportNameValid (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:166:40)
    at validateImport (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:197:13)
    at cb (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:232:21)
    at visitNodes (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/typescript/lib/typescript.js:16631:30)
    at Object.forEachChild (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/typescript/lib/typescript.js:16859:24)
    at walk (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:238:15)
    at Rule.AbstractRule.applyWithFunction (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint/lib/language/rule/abstractRule.js:39:9)

Expected behavior

It should not crash. It should just have a lint error on blahSVG -> blahSvg.

In a debugger, you can see that the resolvedModules values are undefined for non TS or JS files. It fails when accessing. value.isExternalLibraryImport since value is undefined.
image
image

source lines

TS source says that ResolvedModule only picks up TS and JS files


I'm not sure why import fooSvg from './foo.svg'; doesn't crash though. checkIgnoreExternalModule() doesn't seem to be called on names that are named correctly. I haven't read all of your source code, but maybe you guys are doing a basic check for things already named in camelCase.

Another thing is that the crash doesn't count as a lint error so lint still passes.

@IllusionMH
Copy link
Contributor

IllusionMH commented Jul 29, 2019

@dosentmatter thanks for the report.
From static check standpoint it is impossible to tell if import "foo.svg" is import from foo.svg which is not supported by TS itself or from foo.svg.ts which is valid option.

Do you have types.d.ts file or similar with declaration like code below?

declare module "*.svg" {
  // declaration for processed SVG type
}

While Webpack (I assume you use it) can resolve correctly various extension TS doesn't process anything except TS/JS/JSON so this is probably root cause of difference.

Anyway this shouldn't be runtime error.

UPD. Removed incorrect mention of Webpack's resolve.extensions configuration

@dosentmatter
Copy link
Author

Hey @IllusionMH, no problem. I didn't add a types.d.ts to my minimal example, but I just did and it doesn't change anything.

In the original project I was working on, it had a types.d.ts, but still had the error. Yeah, I am using webpack svg loaders for storybook, rollup svg plugins for bundling, and jest svg ignore transforms for tests.

Adding types.d.ts for my minimal example:

declare module "*.svg" {
  const a: string;
  export default a;
}

Here is the type showing in editor:
image

$ npx tslint -p tsconfig.json test.ts
The 'import-name' rule threw an error in '/Users/kevinlau/workspace/test/tslint-microsoft-contrib/test.ts':
TypeError: Cannot read property 'isExternalLibraryImport' of undefined
    at /Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:127:49
    at Map.forEach (<anonymous>)
    at checkIgnoreExternalModule (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:126:48)
    at isImportNameValid (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:166:40)
    at validateImport (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:197:13)
    at cb (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:232:21)
    at visitNodes (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/typescript/lib/typescript.js:16631:30)
    at Object.forEachChild (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/typescript/lib/typescript.js:16859:24)
    at walk (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint-microsoft-contrib/importNameRule.js:238:15)
    at Rule.AbstractRule.applyWithFunction (/Users/kevinlau/workspace/test/tslint-microsoft-contrib/node_modules/tslint/lib/language/rule/abstractRule.js:39:9)

@JoshuaKGoldberg
Copy link

💀 It's time! 💀

TSLint is deprecated and no longer accepting pull requests other than security fixes. See #876. ☠️
We recommend you instead use typescript-eslint to lint your TypeScript code with ESLint. ✅

👋 It was a pleasure open sourcing with you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants