microsoft/vscode-eslint

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):

image

However, in VSCode I only see the issues highlighted within the TS source, not within the inline template:

image

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:

image

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

capture

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:

image

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 -

  1. consider these files:
    "javascript",
    "javascriptreact",
    "typescript",
    "typescriptreact",
    "html"

  2. 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.