Parsing errors when combined with eslint-plugin-vue as of 1.5.0

[thebanjomatic]

PR #367 seems to have introduced a number of unexpected false linter errors when used in combination with eslint-plugin-vue. I think the problem boils down to the assumption from the PR that this function will only get called twice if in watch or fix mode not being valid.

With single-file-components in .vue files, this function winds up getting called for each section of the file that may contain scripts. Naturally this includes the <script></script> block, but also includes things like interpolations and v-bind's, etc. Each time getting called with the same filePath.

What code were you trying to parse?

See the following reproducer:
https://github.com/thebanjomatic/lint-repro

What did you expect to happen?

Running lint should not produce false errors

What actually happened?

Parsing appears to be broken and a multitude of different lint errors are produced.

Versions

package	version
@typescript-eslint/typescript-estree	>= 1.5.0
TypeScript	3.3.4000
node	8.15.0
npm	6.4.1

[MrJmpl3]

I ask too, why?

I solved remove the option extraFileExtensions , I think the problem is here, because with or without this option , eslint typescript work too in vue files correctly !

Example!

Please ignore all dependencies from my personal project.
I think that if I remove something from the package.json, it could affect the result

eslint.js:

module.exports = {
  root: true,
  env: {
    browser: true,
    node: true
  },
  parser: 'vue-eslint-parser',
  parserOptions: {
    parser: '@typescript-eslint/parser',
    project: './tsconfig.json'
  },
  extends: ['plugin:vue/recommended', '@vue/typescript', '@vue/prettier', 'plugin:@typescript-eslint/recommended', 'prettier/@typescript-eslint'],
  rules: {
    'prettier/prettier': ['error', {
      semi: true,
      singleQuote: true,
      printWidth: 150
    }]
  }
};

package.json

{
  "name": "nuxt-ocasani-recave-see-cliente",
  "version": "2.0.0",
  "description": "Proyecto para la empresa RECAVE",
  "author": "MrJmpl3 <josecasani1997@hotmail.com>",
  "private": true,
  "scripts": {
    "dev": "nuxt",
    "build": "nuxt build",
    "start": "nuxt",
    "generate": "nuxt generate",
    "lint": "eslint --ext .js,.vue .",
    "lintfix": "eslint --fix --ext .js,.vue,.ts,.tsx --ignore-path .gitignore .",
    "upgrade-interactive": "npm-check --update"
  },
  "dependencies": {
    "@babel/runtime-corejs2": "^7.4.3",
    "@mdi/font": "^3.5.95",
    "@nuxt/typescript": "^2.6.1",
    "@nuxtjs/axios": "^5.4.1",
    "@nuxtjs/dotenv": "^1.3.0",
    "@nuxtjs/pwa": "^2.6.0",
    "@nuxtjs/robots": "^2.1.0",
    "@nuxtjs/router": "^1.3.2",
    "@nuxtjs/style-resources": "^0.1.2",
    "@types/crypto-js": "^3.1.43",
    "@types/express": "^4.16.1",
    "@types/file-saver": "^2.0.0",
    "@types/helmet": "0.0.43",
    "@types/js-base64": "^2.3.1",
    "@types/node": "^11.13.0",
    "@types/universal-cookie": "^2.2.0",
    "autoprefixer": "^9.5.0",
    "axios": "^0.18.0",
    "chart.js": "^2.8.0",
    "core-js": "^2.6.5",
    "cross-env": "^5.2.0",
    "crypto-js": "^3.1.9-1",
    "dotenv": "^7.0.0",
    "file-saver": "^2.0.1",
    "js-base64": "^2.5.1",
    "laravel-echo": "^1.5.3",
    "moment": "^2.24.0",
    "nuxt": "^2.6.1",
    "nuxt-helmet": "^1.0.1",
    "nuxt-property-decorator": "^2.1.3",
    "perfect-scrollbar": "^1.4.0",
    "purgecss-webpack-plugin": "^1.4.0",
    "socket.io-client": "^2.2.0",
    "stylus": "^0.54.5",
    "stylus-loader": "^3.0.2",
    "sweetalert2": "^8.8.1",
    "thread-loader": "^2.1.2",
    "ts-node": "^8.0.3",
    "typescript": "3.3.4000",
    "universal-cookie": "^3.1.0",
    "vee-validate": "^2.2.0",
    "vue": "^2.6.10",
    "vue-chartjs": "^3.4.2",
    "vue-class-component": "^7.0.2",
    "vue-filter": "^0.2.5",
    "vue-mixin-decorator": "^1.1.1",
    "vue-recaptcha": "^1.1.1",
    "vue-router": "^3.0.2",
    "vuetify": "^1.5.9",
    "vuetify-loader": "^1.2.1",
    "vuex": "^3.1.0",
    "vuex-class": "^0.3.2",
    "webpack-node-externals": "^1.7.2"
  },
  "devDependencies": {
    "@vue/eslint-config-prettier": "^4.0.1",
    "@vue/eslint-config-typescript": "^4.0.0",
    "babel-eslint": "^10.0.1",
    "eslint": "^5.16.0",
    "eslint-config-prettier": "^4.1.0",
    "eslint-loader": "^2.1.2",
    "eslint-plugin-prettier": "^3.0.1",
    "eslint-plugin-vue": "^5.2.2",
    "nodemon": "^1.18.10",
    "npm-check": "^5.9.0",
    "prettier": "^1.16.4",
    "vue-eslint-parser": "^6.0.3"
  }
}

[thebanjomatic]

@bradzacher Its been a few months and this issue is still blocking us from upgrading. I'd be more than happy to submit a pull request reverting PR #367 if you think that would be accepted.

[bradzacher]

TBH I would say that that PR would be rejected outright. #367 improved parsing performance greatly. Considering the vast majority of our users don't use vue, slowing down their performance to fix a bug for a minority is not really an option.

That being said, if someone wants to investigate and PR a fix for this, we'd be ecstatic.
I haven't ever used vue, and don't know how it works, so there is a pretty huge overhead for me to investigate this (also the tsc management portion of ts-estree is something I know little about).
Maybe @uniqueiniquity might have some bandwidth to look into this? He's more of a SME on that part of the project.

---

As an aside, we're all unpaid volunteers, so we don't have a huge amount of bandwidth. I spend most of my open source time (~8 hours a week) managing issues/reviewing PRs on this repo.
We heavily rely upon the community to help us add features, and find/fix issues.

[uniqueiniquity]

Unfortunately, I don't currently have bandwidth to address this. I'd be happy to help review any PRs addressing it, though.

[thebanjomatic]

I can take another look at this myself, but i'm not 100% familiar with the project so I might have some questions along the way.

[thebanjomatic]

After investigating this a little further, I really have no idea what the correct solution is. The current setup seems to (reasonably) operate under the assumption that:

1. there is a 1:1 mapping between filePath and code
2. the source on disk when reading the file is equivalent to code

In the case of vue single-file-components, both of these assumptions break down because each mustache expression within a template gets individually linted, followed by the body of the <script> elements. And there are cases where inspecting the text of the sourceFile reveals that oldReadFile is being called on the vue files and contains the full text including <template>... etc.

The best compromise that I came up with was a bit kludgy, but it selectively removes this optimization if the file has a .vue extension.

  const isVueFile = path.extname(filePath) === '.vue';
  const watchCallback = watchCallbackTrackingMap.get(filePath);
  if ((parsedFilesSeen.has(filePath) || isVueFile) && typeof watchCallback !== 'undefined') {
    watchCallback(filePath, ts.FileWatcherEventKind.Changed);
  }

[bradzacher]

Hmmm... maybe this is something that needs to be changed in vue-eslint-parser instead?
That project is what coordinates and sends code fragments to the "child" parser (i.e. to our parser).

Maybe there is a way they can send fragments to us such that it works without us turning off the performance optimisations?

[thebanjomatic]

Earlier I played around with that idea by appending an incrementing number to the filePath each time it sends a fragment so that they are seen as separate files. The issue there is that the .vue files are loaded from disk when the program is created and are never overridden with valid typescript. This winds up being the source of other errors.

I also tried only modifying the filePath for the mustache blocks, and not for the script block, but I was running into the same basic problems.

[bradzacher]

Ahh I think I get you. Correct me if I'm misunderstanding here - I'm figuring out a bunch of stuff about vue and what ts-estree does as I go...

The problem is that with the change in #367, we switched to relying on typescript to load the files when you provide a project.
Because of this, typescript attempts to parse a vue file, which will obviously never work.

(If I'm understanding correctly)
Hmmm.. I'm not sure if there is a performant solution to this then. The reason we rely on typescript to do the loading and parsing is because it is much more optimised for typechecking and re-checking on changes (it implicitly leverages their caching etc).

To me it sounds like the only solution is to special case vue files to circumvent the current codepath, so that they follow the slow path.
Maybe @mysticatea has another idea because he "owns" vue-eslint-parser.

I'd be curious to see how the vue compiler works with running the typechecker on files and doing performant builds of a project using typescript. Maybe there's some cues we can take from that?

[mysticatea]

Ooh.

I'm not familiar with Vue.js with TypeScript, so I'm not sure if what is right. May vue-eslint-parser have to use espree always to parse mustaches?

[bradzacher]

@mysticatea - I don't know enough about vue to know what's considered valid code...
Can the mustaches contain any valid typescript code, or is it just simple expressions?
i.e. could I do something like {{ (foo as string[]).join(',') }}?

[nothingismagick]

Just an idea - in vue we use webpack loaders - and I have had some success in the past with creating a <test> template in the SFC that creates a Component.spec.ts file and then the CT harness picks up on the changes and runs the tests. Maybe it would be most simple for Vue SFC authors to take the ts block from the SFC and render it to eg. ./ts/Component.ts for typechecking purposes...

[nothingismagick]

I am going to throw together a POC right now.

[IlCallo]

I explored around a bit to see what was already done in this field and who could help.
I found something into Vetur which may be useful in some way, expecially this comment.

To implement interpolations type-checking they apparently did something similar to what @nothingismagick proposed.
The process is briefly explained into the interpolation documentation page.

The main author of the PR was @ktsn, under @octref guidance, so they can probably provide us some insights in what is the best way to fix our problem.

Vetur is also going to have a CLI version and is currently setting the ground by integrating some features directly into TypeScript API.
I don't know if it can be useful, but the scope seems more or less the same, so maybe we can learn something from it.

Here, instead, is the loader @nothingismagick was talking about, which can be used as a base for a vue-sfc-to-ts-loader, if that's the right path to follow.

[IlCallo]

@bradzacher

I don't know enough about vue to know what's considered valid code...
Can the mustaches contain any valid typescript code, or is it just simple expressions?
i.e. could I do something like {{ (foo as string[]).join(',') }}?

From Vetur documentation:

Other than the filter syntax, the expression is 100% JavaScript expression.

From Vue documentation:

Vue.js actually supports the full power of JavaScript expressions inside all data bindings

These expressions will be evaluated as JavaScript in the data scope of the owner Vue instance. One restriction is that each binding can only contain one single expression

Template expressions are sandboxed and only have access to a whitelist of globals such as Math and Date. You should not attempt to access user defined globals in template expressions.

Nothing said about TypeScript syntax (like type casting), but if it is managed at webpack level by separating scripts parts (interpolations and expressions included) and analyzing them with the most appropriate loader, I would guess that it's supported

---

EDIT: tried to add a typecast inside an interpolation, apparently it won't accept type casting

[octref]

I suggest that you only do syntactical linting for template interpolations. And even if you only do that, to properly handle filter you need https://github.com/mysticatea/vue-eslint-parser. Seems the AST is compatible with estree, so you can hook it into typescript-eslint.

If you are interested in how semantical error checking work in Vetur, I wrote about it in more details in http://blog.matsu.io/generic-vue-template-interpolation-language-features.

Actually, why would you need to support template interpolation in typescript-eslint? I thought it's supported by eslint-plugin-vue. I think limiting linting to the <script lang="ts"> section would be good enough.

[IlCallo]

I'm trying to study the problem, but it seems I cannot wrap my head around it.

What does extraFileExtensions parser option actually do?
This is the documentation from parser README:

extraFileExtensions - default undefined. This option allows you to provide one or more additional file extensions which should be considered in the TypeScript Program compilation. E.g. a .vue file

but it's rather vague. I tryed following the white rabbit down the hole and could not understand what's its purpose. Just telling TypeScript in which file extension he'll find TS code? Then it's normal that it fails with .vue files, that's seemingly not how that option is meant to be used.

This statement seem particularly on the point

We switched to relying on typescript to load the files when you provide a project. Because of this, typescript attempts to parse a vue file, which will obviously never work.

Also, quoting the first answer of the issue:

I solved remove the option extraFileExtensions , I think the problem is here, because with or without this option , eslint typescript work too in vue files correctly !

I now fail to see where the actual problem is.
SFC script tag and interpolation are managed by eslint-plugin-vue, as @octref said, leaving you the possibility to apply @typescript-eslint/parser on the script tag.

Is this all about supporting TS into interpolation expressions too?
It doesn't seem a "blocking" issue in any way, you are not supposed to put complex expression there anyway, but I could just have got everything wrong 🤔

[thebanjomatic]

Also, quoting the first answer of the issue:

I solved remove the option extraFileExtensions , I think the problem is here, because with or without this option , eslint typescript work too in vue files correctly !
I now fail to see where the actual problem is.

I didn't find that to be the case... I think when I tested this before without the extraFileExtensions, the vue files weren't being linted at all for me, but I also had an issue where my reproducer didn't have the rules set that I was expecting, but I will test this again. If that is the case, the documentation for how to setup eslint-plugin-vue are incorrect and should be updated.

That being said, I seem to remember my logging implied the files weren't getting hit at all though.

Is this all about supporting TS into interpolation expressions too?
It doesn't seem a "blocking" issue in any way, you are not supposed to put complex expression there anyway, but I could just have got everything wrong 🤔

I don't know if interpolation expressions should be linted or type checked (linting sounds reasonable though) but the problem is that it currently tries to do so in a way that breaks linting for the source files and linting fails completely for my project due to false positives.

[thebanjomatic]

@octref

Actually, why would you need to support template interpolation in typescript-eslint? I thought it's supported by eslint-plugin-vue. I think limiting linting to the <script lang="ts"> section would be good enough.

eslint-plugin-vue currently calls into the typescript-eslint for each interpolation block and also for the script tag with a source string for each individual part being linted. That would be fine if it was only linting the source blocks in relative isolation, and in fact it works if you don't provide a tsconfig.json project file to the linter options.

What winds up happening though is the source files are being read by the typescript project system by just directly loading them from disk... this is fine for .ts files, but not for things like .vue files which need to be transformed before they are valid typescript.

It's actually not the current file that is causing the linter errors, its because when linting part of file A, typescript loads file B from disk and the "typescript" in file B doesn't parse and throws an error.

Brainstorming now, I think ideally there would be some sort of plugin system that eslint-plugin-vue could hook into. There are two basic requirements:

1. We need a way to transform a file path into 1 or more files, this would need to also interact with the implementation of readDirectory so that eslint-plugin-vue can expand a .vue file into multiple fake files for each lang="ts" script-block (assuming there can be more than one) and any interpolations.

2. readFile needs to also be able to load those pseudo-files by name and return the the appropriate block of text.

That way typescript-eslint doesn't need to have vue specific knowledge, but could provide the logic needed to transform .vue files into something the typescript build can load and understand.

[IlCallo]

I didn't find that to be the case... I think when I tested this before without the extraFileExtensions, the vue files weren't being linted at all for me

Maybe it was a bug with in some dependency and now it has been resolved somewhere upstream?
Because I cloned your repro, deleted yarn.lock (because it could not install the locked dependencies), run yarn again, removed the extraFileExtensions options and everything worked 🤔

What you are proposing is more or less pushing the work done by Vetur upstream to vue-eslint-plugin (or into some kind of loader?), am I right?

[bradzacher]

To explain the process a bit more.

If you don't pass in parserOptions.project, then our parser behaves like any other parser with the caveat that it uses typescript to do the parsing of the single file in isolation.

If you pass in parserOptions.project, then we follow a different logic path - we construct a typescript "program" for the provided tsconfig. This causes typescript to parse every file referenced by the tsconfig up-front.

However, eslint won't always work if we only do this, because eslint uses CLI globs to determine files to be parsed. For example if your tsconfig is { includes: ["src/**/*.ts"] }, but your lint command is eslint 'test/**/*.ts', then eslint will pass files that are outside the initial program.

To handle this case, we treat every new file as the root of a new dependency tree, and create a new program root at that file. As this is a new program, typescript will not share cached parses, which means that this is SUPER slow and grows exponentially with each "unknown" file (this is something we're looking into fixing).

In the case of vue, vue-eslint-parser is the one coordinating our parser, and what it does is something along the lines of: parse vue file, gather all script tags and handlebars, iteratively pass those in to our parser with a fake filename. (someone can correct me if I'm wrong with that understanding)

This will work to some degree, however to my understanding you'll have a few problems:

• Because it's giving fake filenames, this means that the fake files will all be outside the initial program, so it'll be very slow.
• Typescript might fall back to parsing in isolated modules mode, which means it won't build a dependency tree, and it won't be able to provide full type information for things (a lot of things will just resolve to any).
  • Because there will be incorrect types, chances are the eslint rules that require type information will fail in unexpected ways.

[thebanjomatic]

In the case of vue, vue-eslint-parser is the one coordinating our parser, and what it does is something along the lines of: parse vue file, gather all script tags and handlebars, iteratively pass those in to our parser with a fake filename. (someone can correct me if I'm wrong with that understanding)

It doesn't actually do the fake file names, it just passes the same filename "BlahBlah.vue" for each of the code sections with a different body text.

I've experimented with the sending fake filenames earlier (#404 (comment)) but the problem was still that the typescript parser was loading the .vue files as if they were typescript because they were included in the project.

[bradzacher]

These should all be fixed now!