Not showing all errors when using ESLint processor on a file
JamesHenry opened this issue · 9 comments
Hi @dbaeumer, thanks for all the fantastic work you do maintaining this extension.
For the first time I am making use of an ESLint processor (I am starting with that context because perhaps there are known issues with processors I have not come across before) in a plugin I am working on for ESLint to support Angular.
The project monorepo can be found here: https://github.com/angular-eslint/angular-eslint
The reason a processor is needed is that an Angular Component is written as a TypeScript file, but can contain an inline template definition (Angular-flavoured HTML within a string).
Issue repro
Using an integration test from the above project as our example, this file shows the inline template I mention and it has some intentional linting issues:
import { Component, OnInit, Output, EventEmitter } from '@angular/core';
@Component({
selector: "app-example",
template: `
<input type="text" name="foo" ([ngModel])="foo">
<app-item ([bar])="bar" ([item])="item" [(test)]="test"></app-item>
<div [oneWay]="oneWay" (emitter)="emitter" ([twoWay])="twoWay"></div>
`,
styleUrls: ['./example.component.scss'],
inputs: [],
outputs: [],
host: {}
})
export class ExampleComponent implements OnInit {
@Output() onFoo = new EventEmitter();
constructor() { }
ngOnInit() {
}
}
My ESLint processor and plugin are successfully working from the command line - I receive both the errors in the TS source code as well as the errors in the inline template reported to me using the Angular CLI builder which behind the scenes is using CLIEngine
(source also available in that monorepo under /builder):
However, in VSCode I only see the issues highlighted within the TS source, not within the inline template:
Note that it is not an issue with HTML in general. because external (non-inline) templates as .html files work fine in VSCode and via the command line:
All you should need to do to reproduce is clone the repo and run yarn
and then yarn integration-tests
, and then open the integration test project at path ./packages/integration-tests/fixtures/angular-cli-workspace
Actually when I validate the file in the terminal using the same eslint version and working directory the ESLint plugin sees I don't get the extra errors eithers. Here is what I did:
- project setup as described by you
- cd into
./packages/integration-tests/fixtures/angular-cli-workspace
- open vscode
- the ESLint output channel tells me that eslint is loaded from
[Info - 11:27:36 AM] ESLint library loaded from: /home/dirkb/Projects/mseng/VSCode/Playgrounds/bugs/angular-eslint/node_modules/eslint/lib/api.js
- running ESlint in a terminal on
angular-cli-workspace
using../../../../node_modules/.bin/eslint src/app/example.component.ts
produces
Could this be a setup/configuration problem on your end ?
I’m not in front of a computer right now but did you include HTML files in your ext?
The processor extracts the template as an HTML file (implementation detail of processor lifecycle).
Not sure what you mean. All I noticed that for me following the instruction you gave me VS Code and the terminal produces the same result.
Back at a computer now. You are not comparing like for like because you are not providing ESLint with ext settings to include HTML, ESLint will not consider HTML by default.
Internally the pre
part of the ESLint processor extracts the template part and creates a kind of fragment file which gets a name and appropriate extension (.html) so that it can be parsed and linted, and then the post
part of the processor ties the location back to the original source file. (The end user is never aware this fragment file ever existed which is what I meant about it being an implementation detail of the processor lifecycle https://eslint.org/docs/developer-guide/working-with-plugins#processors-in-plugins).
Please see here for the behaviour with and without ext using your repro above:
As I included in the original issue description above, the vscode extension does seem to work fine when the root file is HTML, which is what led me to suggest it might be specific to this processor use-case.
Hopefully now the discrepancy is clear, let me know if you need any more info!
Have you tried to use the eslint.options
setting to provide the --ext
option to the CLIEngine. If you have sepecial command line parameter needs you need to pass them to the VS Code extension as well using the eslint.options
setting.
I had previously set:
"eslint.validate": [
"javascript",
"javascriptreact",
"typescript",
"typescriptreact",
"html"
],
I have now also added:
"eslint.lintTask.options": "--ext .ts,.html"
...to match the above.
There is no change in behaviour after restarting VSCode, is it working for you via this or some other editor setting?
The setting you need to use is eslint.options
. Something like
"eslint.options" = {
"ext": ".ts,.html"
}
Ok so given it is being passed directly to CLIEngine, the setting needs to be:
"eslint.options": {
"extensions": [".ts", ".html"]
}
This works! That is great, thank you for helping find a solution.
I would say it is little counter intuitive to have "eslint.validate": []
set with HTML included but then have to set directly related data again in this way just for this processor use-case. I do now get the technical distinction, but doesn't the extension have all the information it needs to apply this automatically?
For example above I am now kind of telling the extension two different things -
-
consider these files:
"javascript",
"javascriptreact",
"typescript",
"typescriptreact",
"html" -
actually consider these files
.ts
.html
I think there will always be this risk of conflicting configs and subtle issues if this isn't incorporated into the extension directly.
Do you see what I mean?
The files types listed under (1) basically denote the files which the VS Code extension should sync to the server to be validated. It is like passing the file to validate when running in the terminal. In your special case there is a sibling file with a different extension that should be validated as well.
I would actually prefer to keep them different for now. I am pretty sure that if I make the suggested change other will complain.
I opened #929 so that the info doesn't get lost.
Closing the issue for now.