Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript-Eslint conflict #1672

Closed
essenmitsosse opened this issue Jun 25, 2018 · 17 comments
Closed

Typescript-Eslint conflict #1672

essenmitsosse opened this issue Jun 25, 2018 · 17 comments

Comments

@essenmitsosse
Copy link

Version

3.0.0-rc.3

Reproduction link

https://github.com/marcus-blaettermann-ventoro/vue-cli-bug/tree/master

Steps to reproduce

Clone the given repo, npm install and then npm run lint

Alternative: Run vue create with those settings:

Vue CLI v3.0.0-rc.3
? Please pick a preset: Manually select features
? Check the features needed for your project: Babel, TS, Linter
? Use class-style component syntax? No
? Use Babel alongside TypeScript for auto-detected polyfills? No
? Pick a linter / formatter config: Airbnb
? Pick additional lint features: Lint on save
? Where do you prefer placing config for Babel, PostCSS, ESLint, etc.? In dedicated config files
? Save this as a preset for future projects? (y/N)

Then add this to the src/components/Hello-World.vue:

enum Direction {
Up,
Down,
}

const foo: Direction = Direction.Up;

What is expected?

The linter should not find any errors, or at least should fix the errors it finds.

What is actually happening?

The linter adds a space before one of the commas in the enum Direction and then complains about it. Removing the space has no effect because the linter adds it the next time again. When using lint on save in an editor, the comma jumps between the first and second line.


  • This has been reproduced on both Mac and Windows.
  • The problem doesn`t occure with the same code in a regular .ts file
  • The problem only occurs, when an enum (something regular JavaScript doesn`t have) followed by something with a type declaration.
  • The problem also occurs, when the enum is in a single line. enum Direction { Up, Down }
  • If the const foo : Direction = Direction.Up; is removed the linter doesn`t mind the enum anymore.
  • I think the linter somehow chokes on the type declaration colon.
@essenmitsosse
Copy link
Author

This also happens when you replace the enum with a simple array declaration - the problem seems to be the type declaration. So the minimal thing to reproduce is just to add something like this in a .vue file:

const bar = [1, 2];
const foo : string = 'hi';

@Akryum Akryum added bug scope: eslint scope: typescript needs team repro We acknowledged your report and will soon try to reproduce it labels Jun 28, 2018
@LinusBorg
Copy link
Member

LinusBorg commented Jul 1, 2018

There's a related issue open in the eslint repo: eslint/eslint#10260

Also, in the repo of typescript-eslint-parser, there seem to be a coupe of issues over time concerning the that same rule, tow of which are still open and likely related:

eslint/typescript-eslint-parser#449
eslint/typescript-eslint-parser#486

So in that context, I would suspect this to be an upstream issue with typescript-eslint-parser for now.

But since this only happens in .vue files (added a .ts file with the same enum, this was linted fine), it's somehow still related to us - possibly vue-loader?

Triage with normal .ts file:

// test.js
enum Direction {
  Up,
  Down,
}

export function dummy() { return null }

importing from this file doesn't raise any linting errors, but it does in a .vue file

Workaround

Adding this entry to the rules in eslintrc is a workaround:

  rules: {
    'space-infix-ops': 'off', // WORKAROUND
    'no-console': process.env.NODE_ENV === 'production' ? 'error' : 'off',
    'no-debugger': process.env.NODE_ENV === 'production' ? 'error' : 'off'
  }

@LinusBorg LinusBorg removed the needs team repro We acknowledged your report and will soon try to reproduce it label Jul 1, 2018
@LinusBorg
Copy link
Member

/ping @michalsnik @mysticatea any ideas?

@mario-d-s
Copy link

This also happens when importing multiple classes from a module:

// Component.vue
<script lang="ts">
import { Foo, Bar } from "Foobar"
...
</script>

produces:

error: Infix operators must be spaced (space-infix-ops)

on the comma between Foo and Bar. Then change it to:

import { Foo , Bar } from "Foobar"

To get:

error: There should be no space before ',' (comma-spacing)

It does not complain on the same style of imports in .ts files.

@mario-d-s
Copy link

I just noticed there must be other problems with typescript in SFC, indentation also doesn't work:

@Component({
    props: {
    info: Object
    }
    })
export default class MyComponent extends Vue { }

I want 4 spaces, but it doesn't indent the info: Object part within props and wants the closing }) to be indented.

@yyx990803
Copy link
Member

Looks like there are some parser related problem when using eslint-plugin-vue in combination with typescript-eslint-parser. I pushed a temporary fix disabling the space-infix-ops rule when using that setup.

@michalsnik
Copy link
Member

Hey guys! Unfortunately I don't have an answer yet, but I'm going to test the whole typescript integration soon and I'll let you know when I know more. For the time being you can disable the failing rule yourself or update the package to get Evan's temporary fix.

@michalsnik
Copy link
Member

I just confirmed this issue, and did some digging in ASTs. Looks like the produced AST for the <script> tag inside .vue file is almost identical to what is produced by plain .ts file. Almost - because one thing differs and causes the problem. In .vue AST there are extra circular parent nodes. In effect - whenever eslint meets VariableDeclarator it'll search through the whole range of tokens inside the file, whereas in .ts there is no parrent to corrupt token search range.

In ESlint v5 the parent node behaviour has slightly changed and now the parent node is being set before we even have access to the AST in eslint rules. I'll try to investigate it a bit more, but it looks like the AST for .vue might be the proper one given those changes and the problem might be in the space-infix-ops rule itself. Also we may have another problem - that plain javascript files are not being parsed consistently if it happens to be true.

I'll post an update once I know more. These are my first insights.

@michalsnik
Copy link
Member

Oh boy, I went through typescript-eslint-parser, eslint and vue-eslint-parser internals, but finally I know what's going on 😅

So:

  • typescript-eslint-parser produces new property typeAnnotation on some nodes in the AST with dedicated token - called TSTypeAnnotation
  • vue-eslint-parser in this scenario uses parser service offered by typescript-eslint-parser to get the enhanced AST
  • one of the responsibilities of vue-eslint-parser, due to the fact that we're parsing .vue files and script tag is usually placed somewhere below template is to update locations and ranges of all tokens in the AST, so that errors produced by eslint point to exact place in our .vue file.
  • Unfortunately vue-eslint-parser doesn't know anything about these new types of tokens, and doesn't update their ranges.
  • space-infix-ops rule happens to handle case with this typeAnnotation, and if it detects this annotation it takes this exact node and based on it's range and range of the former tag finds all tokens that happens to be within the calculated range
  • beacause the token of TSTypeAnnotation type has wrong range - it reports wrongly discovered tokens, hence the error pointed in the description.

I'm going to come up with the fix as soon as I can :)

Fun fact: You can fix it by placing <script> tag at the top of your .vue file :)

@mario-d-s
Copy link

Related issue: eslint/typescript-eslint-parser#438. It is about what I've mentioned in #1672 (comment).

@michalsnik any other issues in the repositories you've mentioned that may be worth cross-referencing?

@michalsnik
Copy link
Member

Not sure @mario-d-s if there's anything more worth cross-referening at this moment.
I created PR in the typescript-eslint-parser where I'm implementing visitorKeys, that will allow our vue-eslint-parser to properly fix locations of all tokens and not throw random errors anymore.
Once it's fixed we'll look further for any problems that might be still relevant.

Regarding indentation, I think this might be relevant topic: vuejs/eslint-plugin-vue#503

Thank you for your active engagement, we really appreciate it @mario-d-s ! :)

@michalsnik
Copy link
Member

Also one more thing about indentation - it looks like it's the problem with native ESLint indent rule, and not vue/script-indent. You should disable the native one in favour of the dedicated we created. Take a look here: https://github.com/vuejs/eslint-plugin-vue/blob/master/docs/rules/script-indent.md#important-note

@mario-d-s
Copy link

@michalsnik sure thing, this must have been quite a headscratcher! Thank you for all the hard work.

It's unfortunate the native indent rule doesn't play nicely with the Vue alternative out of the box, but the workaround is quite acceptable. Good to know and thanks again.

@michalsnik
Copy link
Member

This should work fine now. My PR to typescript-eslint-parser with visitorKeys has been merged and released in v20.1.0 :)

@hiendv
Copy link
Contributor

hiendv commented Nov 9, 2018

@michalsnik I try typescript-eslint-parser v20.1.1 and nothing changes =(

@mysticatea
Copy link
Member

mysticatea commented Nov 16, 2018

Hi.

Recently, I fixed several bugs in typescript-eslint-parser: eslint/typescript-eslint-parser#540

Also, I added the support of typescript-eslint-parser into our online demo: https://mysticatea.github.io/vue-eslint-demo/

Would you confirm whether the problem was fixed or not?

@hiendv
Copy link
Contributor

hiendv commented Nov 19, 2018

@mysticatea I can confirm it's working now. Thank you.

greenhat616 pushed a commit to hitokoto-osc/vue-aplayer that referenced this issue Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants