Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Directories support #857

Closed
wants to merge 2 commits into from
Closed

Directories support #857

wants to merge 2 commits into from

Conversation

louy
Copy link
Contributor

@louy louy commented Dec 6, 2015

Accepting directories as arguments in cli. For example tslint .

Related: #692

Note: node_modules and typings are now ignore by default, even in subdirectories.

@blakeembrey
Copy link
Contributor

Just some comments since I'm interested in such a feature, is it possible to make tsconfig.json a "mode" and have exclude be a CLI flag which overrides the default excludes?

@louy
Copy link
Contributor Author

louy commented Dec 15, 2015

yeah sure. something like --project, -p?

@blakeembrey
Copy link
Contributor

That makes sense, so it'll be opt in? I more need something that looks like tslint "src/**/*.ts" --exclude "__test__/fixtures/**". If you don't want this going way out of scope though, I can make another PR. It's just using tsconfig.json as the source of truth isn't really correct all the time, plus promoting using exclude might confuse people (the compiler will use files and ignore exclude according to the spec, but there's no such check here).

glob.sync(file).forEach(processFile);
}
const fileArgs: string[] = argv._;
const files: string[] = fileArgs.reduce((list: string[], arg: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for : string[] typedef

@adidahiya
Copy link
Contributor

Filed #915 to track --exclude.

@adidahiya adidahiya added this to the TSLint 3.x milestone Jan 15, 2016
@andrenarchy
Copy link

Great feature! This brings tslint closer to the behavior of other popular linters, e.g., eslint. Is there any progress on the revision?

@adidahiya
Copy link
Contributor

ping @louy -- will you get back to this PR or should someone else take up this feature?

@louy
Copy link
Contributor Author

louy commented Feb 6, 2016

@adidahiya didn't you say that we should wait for #858 before moving on with this one?

TBH I'm thinking of a different approach now. here's how I suggest the workflow to be:

  1. if you pass tslint command a file, it lints it.
  2. if you pass tslint a directory, it looks for a tsconfig.json file.
    1. if there's a file, it reads the files prop and only lints them
    2. if there's a file and it has an exclud option, it lints all files that match **/*.ts in that folder except those specified in exclude.
    3. if there's no tsconfig file, it lints all files that match **/*.ts.
  3. when searching a directory for files, tslint should look for a .tslintignore file. if that file is found, it ignores everything that matches rules inside that file even if they're listed in tsconfig.json#files. that file should have the same syntax as a .gitignore or .eslintignore file.
  4. when linting a file, tslint should look for all .tslintrc files in the same and parent directories until it reaches a file which has the "root": true option in it. it should merge all .tslintrc files found with the deepest file having the highest priority.

so in order to exclude .d.ts files in a project, all you have to do is create a .tslintignore file with *.d.ts inside it. you can also add typings/ to ignore all files in typings folder.

this is how I imagine tslint should work ideally, and I think this is how eslint and jshint work.

@blakeembrey
Copy link
Contributor

@louy I don't think tslint should use files unless it intends to completely replicate tsc behaviour and start working in a project context. It's a lot of work and the things I use in tsconfig.json are never representative of what I want to lint - so it's implicit behaviour I'll never use and will likely confuse even though I've read this issue. Also, you need to resolve .tsx. On top of that, what's the behaviour with allowJs enabled?

@unional
Copy link
Contributor

unional commented Mar 31, 2016

It's a lot of work and the things I use in tsconfig.json are never representative of what I want to lint

I hold a slightly different opinion. For tsconfig.json that is consumed by tsc, yes I agreed.

But for tsconfig.json that is consumed by ts language service, it is likely what I want to lint also.

Of course, able to decouple them is a good thing. So how about:

  • Read from tslint.json/fileGlobs if exists
  • If not, then read from /tsconfig.json (or add a field tslint.json/tsconfig to point to the file)

Just saying on top of my head. 😏

@blakeembrey
Copy link
Contributor

@unional filesGlob is non-standard and the first sentence I wrote is the most important here - if tslint doesn't create a project instance and resolve everything as tsc does then it's not really useful to work with files. The files field in tsconfig.json is meant as entry points for the compiler to start resolving and using it to mean something else sounds like a frustrating exercise to me.

Isn't TypeScript language services is the same as tsc here? I'm not 100% sure about the comment, both tsc and the language services resolve the dependency tree which is why it'd be important that tslint start working in a "project" basis, not a file basis.

@unional
Copy link
Contributor

unional commented Mar 31, 2016

if tslint doesn't create a project instance and resolve everything as tsc does then it's not really useful to work with files. The files field in tsconfig.json is meant as entry points for the compiler to start resolving and using it to mean something else sounds like a frustrating exercise to me.

Agree~~ But as mentioned #857 (comment), I thought we are talking about project here. 😄

Isn't TypeScript language services is the same as tsc here?

Code-wise, yes. Application-wise, no.
tsc is about compilation. TypeScript language service is about type resolution and provide support to IDE.
That's why I have been saying they should consume different tsconfig.json.
tsconfig.json should use files to explicitly point out what to compile. They should be:

  • source files
  • typings files

tsconfig.json should use exclude (or include) to include ANY files the user may read and edit. These includes:

  • source files
  • test files
  • plumbing files
  • typings files
  • etc.

@adidahiya
Copy link
Contributor

Closing this PR since it's very out of date. Let's continue discussion in #692 if necessary.

@adidahiya adidahiya closed this Oct 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants