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

Type checking for template expressions #209

Closed
ktsn opened this issue May 16, 2017 · 33 comments · Fixed by #681
Closed

Type checking for template expressions #209

ktsn opened this issue May 16, 2017 · 33 comments · Fixed by #681

Comments

@ktsn
Copy link
Member

ktsn commented May 16, 2017

Hello, do you have any plan to implement type checking/diagnostics for template expressions like angular's language service?
It's probably difficult to implement but it would significantly improve dev experience.

I would like to work for it and would be great to collaborate with you if you like it. What do you think?

@octref
Copy link
Member

octref commented May 16, 2017

I do have it planned. The way it works would be:

  • Use

    jsLanguageService.getProgram().getTypeChecker().getContextualType(defaultExportExpression)

    to get the contextual type of the exported object in *.vue files

  • Extract the template expressions, either in html attributes or in {{}} and type check them.

  • Send diagnostics/errors to VSCode

Current problems:

  1. props is not yet typed. @DanielRosenwasser is working on the type definitions
  2. There needs to be a way where html Language Service can ask js Language Service for the type of props, data, etc.

There is a lot of refactoring and groundwork to be done, but I'm busy studying the finals.

Let me clean it up next week and ping you back.

Meanwhile, maybe you can try building the project according to Contributing doc, and let me know any problem you have.

One thing you can start looking into is how to evaluate the template expressions like {{name}}, where name can be props / data / methods on this. We can evaluate {{this.name}} directly, but we need to find a way to evaluate {{name}} based on the properties of this.

Good reads:

For the code, take a look at

Looking forward to collaborating with you 😺

@ktsn
Copy link
Member Author

ktsn commented May 17, 2017

Thanks for sharing the information 🙂
OK, I'll looking into about the template expressions evaluation.

@ktsn
Copy link
Member Author

ktsn commented Jun 5, 2017

For template expression checking, I ended up to create a naive type checker.
WIP source code is here: https://github.com/ktsn/vue-template-diagnostic

But maybe, there is a simpler approach to do that (as I do not understand typescript internals so much). What do you think?

I tested it with some simple expressions but it probably does not work in practical cases yet. I'll try to improve it with more test cases and to integrate it with vetur if the approach is fine.

@octref
Copy link
Member

octref commented Jun 6, 2017

Thanks a lot for the hard work!

The overall approach seems fine. Now what's remaining:

  • Extract the type info of the default export
  • Create SimpleSymbolTable out of it
  • Share it with vue-html Language Service
  • Run html template expressions against vue-template-diagnostic and surface the diagnostics

I'll play around with vue-template-diagnostic a bit and let you know. Overall solid start 👍

@DanielRosenwasser FYI and curious what's your progress on the typed props.

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Jun 6, 2017

vue-template-diagnostic almost implements a checker from scratch. That's the same approach with Angular. The drawback is incomplete type checking. For example, Angular Language service can detect member.nonExsistingMethod is error, but member.someMethod(wrongArgumentType) will not be caught. (plus, reimplement overload is nightmare)

I'm considering another approach. For all expressions in vue template, compile them into equivalent TypeScript, in an temporary ts file, and get checked by tsc. It waivers much reimplementation, but probably not enough space for customization.

@ktsn
Copy link
Member Author

ktsn commented Jun 6, 2017

I also thought about that approach but I was not sure how we can achieve it in some complex cases.

In my thought, we could type check template expressions if we would add variable declarations that are equivalent with the corresponding component members. But I have no idea how we can declare them in some cases. For example, there are variables that use a private type in an external library declaration. I think it would be nice if we can implement it, though.

@DanielRosenwasser
Copy link
Contributor

FYI and curious what's your progress on the typed props.

I've had some ideas about this; problem is that Vue's API has no real place to put the information for anything complex.

@DanielRosenwasser
Copy link
Contributor

But for what it's worth, many things do actually work:

Vue.extend({
  props: {
    foo: Number, bar: String
  },
  methods: {
    blah() {
      // Should work perfectly
      this.bar.toLowerCase();
    }
  }
});

@ktsn
Copy link
Member Author

ktsn commented Dec 10, 2017

@octref
I'm recently working for this with @HerringtonDarkholme's approach which transforming Vue template into equivalent TS code and I found it's probably possible.

The current problem is that we may need to extend Vetur's html parser so that we get expression locations and parse interpolated expressions for accurate error reporting. We could directly transform template AST to TS AST with original text location info if we have more accurate template parser.

I also found @mysticatea is doing a great job for Vue template parser (https://github.com/mysticatea/vue-eslint-parser) which we could use in Vetur for that purpose. But I'm not sure how much effort is needed to migrate current parser to vue-eslint-parser.

What do you think about that? I think it's also ok to improve the current html parser but I just want to know your idea to avoid mismatching with the Vetur's roadmap that you are thinking.

@octref
Copy link
Member

octref commented Dec 11, 2017

Are you using vue-template-compiler to get the AST and then map it to a TS AST?

I'm also thinking about replacing the html parser. However, one important thing for a LS parser is error-tolerance -- when user is writing code, most of the time the code would not be in valid state. The parser needs to generate meaningful AST, with which we can do analysis.

I was looking at reshape / parse5 / htmlparser2, but haven't decided on anything. vue-eslint-parser might be an option, but I guess it's mostly designed to catch semantic errors in Vue template. Don't know how the AST would look like for incomplete code -- I can take a look though.

But ultimately, we can have multiple parsers anyway, as we are already using eslint-plugin-vue.

@mysticatea
Copy link
Member

Hello. Thank you for the mention about vue-eslint-parser.

I'm also thinking about replacing the html parser. However, one important thing for a LS parser is error-tolerance -- when user is writing code, most of the time the code would not be in valid state. The parser needs to generate meaningful AST, with which we can do analysis.

I agree.
People sometimes use ESLint in their editor, so robustness is important for vue-eslint-parser as well. I'm happy if you teach me about that. Currently, vue-eslint-parser has the recovery logic that HTML spec defines, but I have realized that vue-eslint-parser has some annoying cases if the code has an eof-in-tag syntax error.

For example:

<template>
  <div>
    <div class="
  </div>
</template>

The value of this class attribute is \n </div>\n</template> and have an eof-in-tag error 😢

I was looking at reshape / parse5 / htmlparser2, but haven't decided on anything.

Me too 😄
In more exactly, I had used parse5 at first. However, Vue.js template has many differences from HTML, then I have made from scratch eventually.


By the way, vetur is using eslint-plugin-vue, so maybe you can take the AST of vue-eslint-parser from ESLint CLIEngine instance.


@HerringtonDarkholme @ktsn I'm interested in the type information. Can I use the type info to check unknown elements and undefined properties in eslint-plugin-vue layer?

@ktsn
Copy link
Member Author

ktsn commented Dec 12, 2017

Oh, I didn't notice that I can take the AST from CLIEngine instance. I'll look into it and make vue-eslint-parser AST to TS AST transformer, thanks!

@mysticatea
Probably you can. If the script block exports Vue.extend or class style component (e.g. using vue-class-component), you can use TypeScript type checker to get the component type and its property types.

I had wrote such code in the past and it may help you to grab how you use TypeScript compiler API. https://github.com/ktsn/vue-template-diagnostic/blob/master/src/component-host.ts#L9-L44

If the script block exports an object literal, you need to wrap it with Vue.extend to enable type inference like Vetur does. https://github.com/vuejs/vetur/blob/master/server/src/modes/script/preprocess.ts#L62-L94

@HerringtonDarkholme
Copy link
Member

Can I use the type info to check unknown elements and undefined properties in eslint-plugin-vue layer?

The largest concern is how can we cover enough cases. Some common usage coming to mind is like "global mixin", "component inheritance" and "event emission". Those cases are hard to handle even in a static typed language.

I wonder if it is feasible to give a good error reporting UX.

@octref
Copy link
Member

octref commented Dec 14, 2017

@mysticatea Another similar case I found is

<template>
  <div>
    <div
  </div>
</template>

But to be fair, htmlparse2 generates a similar AST and do not report errors...

Here is a PHP parser that's written from scratch, because none of the existing PHP parsers are error-tolerant enough for editing scenario: https://github.com/Microsoft/tolerant-php-parser

There is a lot of docs about the design and approach in that repo.

I don't know if converting vue-eslint-parser would be easy, and I think it would be out of scope for it since the error-tolerant would be a parser best fitted to catch HTML syntax errors, doing completion suggestions etc.

@ktsn - It's fine if you just use vue-eslint-parser for now. One day Vetur could have a HTML parser that gives you location info for expressions and we can switch later.

@HerringtonDarkholme - We can always enable it via a setting like vetur.experimentalTypeChecking and keep improving it for wider cases.

@octref
Copy link
Member

octref commented Dec 14, 2017

Just a side note, I think vue-eslint-parser might be best for prettier-style formatting for Vue template since it parses the interpolation expressions too.
@lbogdan

@mysticatea
Copy link
Member

Thank you so much!

@ktsn Thank you for pointing the code to learn TypeScript compiler API. It's the thing I want to learn.

@HerringtonDarkholme That's true. I have the same concern since eslint checks source code per each file. I had hoped the type info has a hint to solve the problem. Thank you for the answer.

@octref Thank you for sharing the example and pointing to tolerant-php-parser. I will read it.

@octref
Copy link
Member

octref commented Jan 31, 2018

Just as a note to myself...Need to support filter too which is not standard JS.

@victorgarciaesgi
Copy link

Is there any update on this? Or can we contribute?

I'm working with typescript no maybe it's easer to communicate with the typescript service.
Also webstorm can get the props so maybe we take a look at their work!

@chriszrc
Copy link

chriszrc commented Dec 8, 2018

This is an auxiliary question, but I'm wondering not about type checking in the ide, but at least during vue's aot compilation. In angular for instance, I'd get errors if a variable referenced in a template was undefined, but afaik, this kind of check isn't happening in vue? Am I missing something, or is it not supported?

@octref octref added this to the March 2019 milestone Feb 9, 2019
@lake2
Copy link

lake2 commented Mar 8, 2019

I need this function. Any update of progress?

@octref octref modified the milestones: March 2019, April 2019 Apr 2, 2019
@irissoftworks
Copy link

I come from an Angular background, but am really keen on giving Vue a go, since it seems to have such a great community, good tooling and a nice way of working.

This feature is really killing me though. To clarify, same as @chriszrc, for us it's not so much about IDE support but the build itself that really matters, so that our CI can capture these errors.

I am a bit afraid not having this feature will instil the fear of refactoring into our team which is a big no no for the way we operate.

@sobolevn
Copy link

@octref @ktsn is it possible to encapsulate this logic into some eslint / typescript plugin / standalone tool to run these checks in CI?

It would be a huge bonus! Since right now I have almost everything typed in my https://github.com/wemake-services/wemake-vue-template expect for the template. And that's where a lot of errors happen.

The sad part is that not all developers use vetur, some of them use different editors, or just missing these errors. And some of them are almost impossible to find.

Since templates do not have any coverage information, it is hard to tell what is covered, and what is not. We have snapshot tests that really make a difference, but sometimes errors are hiding in the uncovered conditions.

Is there anything I can help you with?

@octref
Copy link
Member

octref commented Jun 28, 2019

@sobolevn TS Server Plugin has limitations so it's tough.

https://github.com/microsoft/TypeScript/wiki/Writing-a-Language-Service-Plugin#whats-a-language-service-plugin

Plugins can't add new language features such as new syntax or different typechecking behavior

Did you give other VLS based editor plugins a try? I know there are sublime/vim/atom plugins using VLS.

@sobolevn
Copy link

While LS plugins are great for editor tooling, they still do not solve the most important task: ensuring correctness of the system.

eslint (/ etc) plugin seems like a good idea to me. How hard is that to extract this logic?

@octref
Copy link
Member

octref commented Jun 28, 2019

You need to wait for vuejs/rfcs#64 (comment) and #1149

@sobolevn
Copy link

@octref awesome news! Thanks for sharing!

@bugproof
Copy link

bugproof commented Aug 1, 2019

It's closed. Does it work?

image

@sobolevn
Copy link

sobolevn commented Aug 1, 2019

@wrathofodin yes, check out https://github.com/wemake-services/wemake-vue-template

@bugproof
Copy link

bugproof commented Aug 1, 2019

@sobolevn but I want to have this in my existing project. I don't understand how this template can help?

@sobolevn
Copy link

sobolevn commented Aug 1, 2019

It contains the required configuration and the reproduction that this feature works. So that you can use it in your project. It is also the fastest way to demonstrate that this feature works and for you to try. That's why I suggested it.

@bugproof
Copy link

bugproof commented Aug 1, 2019

Is it hard to enable it?

@bugproof
Copy link

bugproof commented Aug 1, 2019

@sobolevn tried your template and it doesn't work there as well (I'm on windows btw.)
image

@winuxue
Copy link

winuxue commented Aug 1, 2019

this doesn't work on workspaces. It says:

This setting cannot be applied now. It will be applied when you open this folder directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.