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

Add an importNameRule option to ignore npm modules #541

Closed
lonyele opened this issue Oct 11, 2018 · 4 comments · Fixed by #569
Closed

Add an importNameRule option to ignore npm modules #541

lonyele opened this issue Oct 11, 2018 · 4 comments · Fixed by #569
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Rule Feature Adding a feature to an existing rule.
Milestone

Comments

@lonyele
Copy link
Contributor

lonyele commented Oct 11, 2018

I've used "tslint-config-airbnb" and it uses this library. I think ImportNameRule can be useful when It checks users code, not npm installed module's code.

I recently tested with my new project setting and
import styled from "styled-components" and got tslint warning "Misnamed import. Import should be named 'styledComponents' but found styled"

I made this rule to be false, and it is all find. Just curious about this result

@cyberhck
Copy link
Contributor

cyberhck commented Oct 13, 2018

I might be way wrong here, but can't you just do, import styledComponents from "styled-components"? I thought it's just a default export, and normally in TypeScript I've mostly used something like import * as styled from "styled-components", maybe that's a way?

@lonyele
Copy link
Contributor Author

lonyele commented Oct 13, 2018

I tried it import * as styled from "styled-components" because that is what I'd usually done for a react or other modules. Then I get this error message when trying to use

styled.h1` some styled-components css here `

[ts]Property 'h1' does not exist on type 'typeof import("c:/Users/roniel/Documents/LearningByExamples2/test-project-setting/node_modules/@types/styled-components/index")

`

@JoshuaKGoldberg JoshuaKGoldberg added the Status: Needs More Info The issue, as stated, needs additional clarification and context. label Oct 14, 2018
@JoshuaKGoldberg
Copy link

JoshuaKGoldberg commented Oct 14, 2018

Hey @lonyele, thanks for posting this! Agreed that it would be nice to have an option for the rule to ignore npm modules.

The import-name already allows passing an object to describe a whitelist of npm modules and their imported names:

// tslint.json
{
    "rules": {
        "import-name": [true, {
            "styled-components": "styled"
        }]
    }
}

You can try that kind of configuration until the option is added. Since the rule already takes in a general object as that mapping, maybe it should also be able to take a string "ignore-node-modules"? Example:

// tslint.json
{
    "rules": {
        "import-name": [true, "ignore-modules"]
    }
}

As for your error message, that looks like a TypeScript complaint, not TSLint. Maybe it's this? https://stackoverflow.com/questions/32236163/when-to-use-import-as-foo-versus-import-foo

@JoshuaKGoldberg JoshuaKGoldberg changed the title importNameRule with npm packages Add an importNameRule option to ignore npm modules Oct 14, 2018
@JoshuaKGoldberg JoshuaKGoldberg added Status: Accepting PRs Good First Issue 🙌 Howdy, neighbor! Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Type: Rule Feature Adding a feature to an existing rule. Hacktoberfest and removed Status: Needs More Info The issue, as stated, needs additional clarification and context. labels Oct 14, 2018
@lonyele
Copy link
Contributor Author

lonyele commented Oct 15, 2018

@JoshuaKGoldberg It sounds nice. I have other things to do right now so I can't make a PR(Actually, I already saw the codebase, but I couldn't really understand all of it to make a useful PR)
Yeah, definitely It is in my learning list and hopefully I make a PR

JoshuaKGoldberg pushed a commit that referenced this issue Oct 24, 2018
#569)

* Make more arguments can be passed to options by using index flag and make more state for the later

* Extract checking replacements logic to function for more checking for the later

* Extract ignoredList from third argument

* Make "isImportNameValid()" return true when ignoredList has moduleName that is currently in a validation process

Add "es7" to "lib" at tsconfig.json for using Array.includes() with string[] type

* Extra config from fourth argument

* Ignore if isExternalLibraryImport flag is true from runtime node object

* Add Tests that can be tested with "grunt all"

- Need Help
I want to write a test about ignoring node_moduels feature, but "node" object is only given at runtime. I don't know how to write a test of it.

* for.. of doesn't support Map at es5 target. It just changes to for loop and silently doesn't go into for loop. I changed it to forEach. Now It's working.

* 1. Remove "es7" from tsconfig
2. Without es7 support, Array.includes doesnt work. I changed it using filter function. (This is the reason why I added es7 support)  I think Array.includes() is not that 'absolutely needed' I just removed it.

* comment added?

* comment for interfaces

* Add optionExamples

* Change optionsExamples
--> naming of previous one can be confusing.
@JoshuaKGoldberg JoshuaKGoldberg added this to the 6.0.0-beta0 milestone Nov 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Rule Feature Adding a feature to an existing rule.
Projects
None yet
3 participants