Skip to content

Conversation

tomastrajan
Copy link
Contributor

This works for the described use cases like with lint-staged or anything else which expects ng lint to accept files as args similarly to tslint.

ps: my first PR ever so be kind thank you!

@tomastrajan
Copy link
Contributor Author

Link to original issue.

@filipesilva
Copy link
Contributor

@Brocco can you review and see how it fits into #7612?

if (args.length) {
configs.forEach(config => {
config.files = args;
config.project = '';
Copy link
Member

Choose a reason for hiding this comment

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

This disables any rule that requires type information. Unfortunately, the underlying issue being worked around here of specifying files that are not part of the program is a blocker for this feature.

I think some further thought on the semantics of passing files on the command line and the interaction with the defined configurations is needed. As well as a more general assessment of the desired behavior when files are specified either in the configuration or command line that are not present within the program (behavior could be different). TSLint has a pending PR that issues an explicitly worded fatal error when this condition occurs. My suggestion would be an additional option that filters out the errant files. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a very good point. Previously I was not aware of tslint using type information for evaluating of some rules, thanks!

Another way to implement this which I was considering is to pass files array deeper where the files are being evaluated and just use them to filter what was retrieved. So if I pass file foo.ts as args... It will only be used to remove everything which is not foo.ts but it will not add itself. This should prevent issue when typescript compilation fails as with the issue described in tslint repo.

What are your thoughts?

Choose a reason for hiding this comment

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

How about only filtering the output by the files names that we want to have information ?

Like ng lint services ->

Do linting to all the files and report only errors on the files in the filter ?

@Nxt3
Copy link

Nxt3 commented Jan 24, 2018

Any update on this?

@hansl hansl unassigned Brocco Feb 8, 2018
@hansl
Copy link
Contributor

hansl commented Feb 22, 2018

Closing this in favor of the Architect effort; you can now specify which app/lib you want to lint.

@hansl hansl closed this Feb 22, 2018
@Nxt3
Copy link

Nxt3 commented Feb 22, 2018

@hansl That's not useful if you want to use libraries like lint-staged--like the OP stated.

#7612

@scott-ho
Copy link

scott-ho commented Mar 8, 2018

@hansl closing the PR seems to be not reasonable. What the users want is perform linting on target files, but not particular sub projects.

@akrueger
Copy link

akrueger commented Apr 1, 2018

@hansl The main reasons this feature is requested are:

  • Linting entire apps/libs/projects is very resource/time intensive
  • It's valuable to lint files before committing them

Your rationale for closing this is a solution that shaves millimeters off of a meter long issue.

I was able to achieve the functionality I wanted by going directly through tslint with this:

"lint-staged": {
    "*.+(ts|js|json)": ["prettier --write"],
    "*.ts": ["tslint -p ./client/tsconfig.json"],
    "*.+(ts|js)": ["jest --findRelatedTests --bail"]
}

The -p <tsconfig.json PATH> portion is for type-checking rules

#7612

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants