Angular-cli: Limit lint to changed files

[filoxo]

Bug Report or Feature Request (mark with an x)

- [ ] bug report -> please search issues before submitting
- [x] feature request

Desired functionality.

Currently, ng lint analyzes all files in the project. I work on a project that has ~260 .ts files in a single project (multiple modules), and to lint all of them takes a while and the output can be hard to digest if there are errors across many files. I think a feature flag (ng lint --staged or similar) to lint only the files that have changed would be a great addition. This is a pretty common feature (eg. lint-staged).

This would also help another specific use case: if a dev/team wants to run lint as a precommit hook, linting just the changed files would make a lot of sense without the burden of analyzing extra files.

Alternative

I could see an argument for not incorporating this into the CLI because it would either be dependent on a single version control system or have support all of them (maybe just git at first, others incrementally). A great alternative would be to expose a --files flag so that consumers can have control over which files are linted. This way would work easily with lint-staged. Below would be an example of how a sample config inside package.json might look (from lint-staged docs):

{
  "scripts": {
    "precommit": "lint-staged"
    ...
  },
  "lint-staged": {
    "src/**/*.ts": ["ng lint --fix", "git add"]
  }
}

The lint-staged docs seem to suggest that tslint already works but the way the CLI retrieves files doesn't seem to make this possible at the moment. Would love to be wrong! If I am wrong, perhaps it could be added to docs in some way showing how it could be done.

[filipesilva]

Would be a nice feature to have, I agree.

[tomastrajan]

Hey guys !

I am currently looking into this and the version in which ng lint just accepts list of files passed by lint-staged seems pretty ok with minimal change to the code and no need to introduce additional ng lint flag. Only thing needed is just to accept args and pass them into configuration which already supports files property.

[tomastrajan]

While implementation is short and works great with demo repo and lint-staged, I am still fighting with writing tests mostly because of the tslint issue and a way the e2e tests are set up. Hopefully I can find solution soon.

[gajdot]

Any workaround or advance on this front? It's really painful to wait for the whole project to be linted.

[rot26]

@gajdot my workaround is to use tslint

tslint --fix $(git ls-files --modified)

[Nxt3]

@rot26 tslint doesn't use codelyzer so it's unfortunately not the exact same.

Since https://github.com/angular/angular-cli/pull/8052 was closed, I guess we're never getting this feature.

[seanlon]

Hi guys,

my approach ..hope it helps
I) configure command to trigger node program during precommit
II) invoke a node program that invoke commands
III) use commands of git to check what is staged files
IV) using the file list, trigger command of ng lint to check each of that specific file

https://github.com/seanlon/demo-angular-app/tree/feature/precommit-partial-stage

http://angular-friday.com/2018/04/24/pre-commit-hook-angular-lint-check-specific-files-committed-stage/

[scott-ho]

@seanlon your approach is just deceiving yourself. It still runs the full lint and just filters out the staged files. That not what we want and will have poor performance for large project.

[scott-ho]

with the cli v6.0.0, we might be able to achieve the goal. The tslintBuilder supports tslint files input https://github.com/angular/devkit/blob/v6.0.0-rc.5/packages/angular_devkit/build_angular/src/tslint/index.ts#L172.

We have to add new params for lint command to pass the input to tslint.

[seanlon]

@scott-ho , as dev of the angular, my perspective is to reuse whatever we can within the Dev cli ecosystem..to prevent reinvent wheel..

As you pointed out the via Ng lint filter is "That not what we want and will have poor performance for large project." then i think could just hack around tslint.. or wait it to be tuned for performance standard.

: )

[bantisk]

pre-commit git hook worked for me on windiows. My package json file.

"config": {
"ghooks": {
"pre-commit": "@powershell -NoProfile -ExecutionPolicy Unrestricted -Command node node_modules/tslint/bin/tslint -e package.json $(git diff --name-only --cached)"
}
}

[istvandesign]

Any updates ? This is really annoying for any real project, I first have to stash all my changes and mocks before pushing a single file.

[johannesjo]

@istvandesign v7.1.2 now allows for specifying the --files parameter. It's not perfect though. As you have to manually specify the tsconfig.app.json.

See: https://angular.io/cli/lint

Edit: Was unable to make it work for more than a single file though.

[PowerKiKi]

A quick test with something like the following goes from ~50 seconds down to 11 seconds for a single file. It's still not very fast, but much more reasonable.

ng lint --ts-config src/tsconfig.app.json --files src/app/my-file-to-test.ts

Is anyone aware of any downside to this technique ? will it come up with exactly all the same errors than a full pass ?

[tao-cumplido]

@johannesjo
You can use the --files flag multiple times, e.g.

ng lint --files=file1.ts --files=file2.ts

I made myself a little helper script to convert a lint-staged call to the given npm script with --files flags.

ng-lint-staged.js

const { exec } = require('child_process');

const cwd = process.cwd();

const script = process.argv[2];

const files = process.argv
    .slice(3)
    .map((f) => {
        // ng lint's --files argument only works with relative paths
        // strip cwd and leading path delimiter
        return `--files="${f.replace(cwd, '').slice(1)}"`;
    })
    .join(' ');

exec(
    `npm run ${script} -- --tsConfig=tsconfig.json ${files}`,
    (error, stdout) => {
        if (error) {
            console.log(stdout);
            process.exit(1);
        }
    }
);

And lint-staged calls the script in the following configurations

"src/**/*.ts": [
    "node ng-lint-staged.js lint:app",
    "git add"
],
"e2e/**/*.ts": [
    "node ng-lint-staged.js lint:e2e",
    "git add"
],

[nuclearglow]

We also ran into this issue with @angular/cli 7.2+. The solution above did not work for us since we do not use lint-staged, so I implemented a small script that directly runs ng lint and finds out for itself the changed files. For that, I have installed the devDependency staged-git-files:
npm i -D staged-git-files.

Here's the script:

const { execSync } = require('child_process');
const stagedGitFiles = require('staged-git-files');

stagedGitFiles((error, stagedFiles) => {
    if (error) {
        console.error(error);
        process.exit(1);
    }
    if (stagedFiles && stagedFiles.length) {
        const stagedAngularTSFiles = stagedFiles.filter(
            (file) => file.status !== 'Deleted' && file.filename.startsWith('src') && file.filename.endsWith('.ts')
        );
        if (stagedAngularTSFiles.length) {
            let ngLint = 'ng lint angular-frontend --fix ';
            for (const { filename } of stagedAngularTSFiles) {
                ngLint = ngLint.concat(`--files ${filename} `);
            }
            console.log(`Linting ${stagedAngularTSFiles.length} staged Angular TS files...\n`);
            execSync(ngLint, { stdio: 'inherit' });
        } else {
            console.log('No Angular TS files staged...\n');
        }
    } else {
        console.log('No Angular TS files staged...\n');
    }
});

In package.json, I have added this to the husky pre-commit hook job list. Seems to work well so far and cut down our waiting time by a lot. Suggestions and feedback welcome :-)

[jakubjosef]

This is bash-only version.
git diff --cached --name-only | grep -E '\.(js|ts)$' | xargs printf -- '--files=%s\n' | xargs ng lint -- --tsConfig=tsconfig.json 2>/dev/null

If you want to skip e2e (as we using it this way) you should specify your project name after ng lint.

[filoxo]

Seems like there are some community solutions to this, and there hasn't been enough interest in incorporating into the actual CLI. Closing.

[jakubjosef]

@filoxo I agree, but it will be nice if you can at least remove this warnings when you use multiple --files args as @tao-cumplido offering.

Option "files" was already specified with value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts"]. The new value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts"] will override it.
Option "files" was already specified with value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts"]. The new value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.ts"] will override it.
Option "files" was already specified with value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.ts"]. The new value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.ts","src/app/iam/api-credentials/api-credentials.module.ts"] will override it.
Option "files" was already specified with value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.ts","src/app/iam/api-credentials/api-credentials.module.ts"]. The new value ["src/app/iam/api-credentials/api-credentials-container.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.spec.ts","src/app/iam/api-credentials/api-credentials.component.ts","src/app/iam/api-credentials/api-credentials.module.ts","src/app/iam/iam-table-settings/credentials.table-settings.ts"] will override it.

[michaelz]

Also, I wasn't able to provide an array of elements to the --files flag. Is there a specific format I should use ? The parser doesn't understand commas nor spaces.

[angular-automatic-lock-bot[bot]]

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._}