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

Allow applying templateInterpolationService only to lang="ts" components #1427

Open
jackkoppa opened this issue Sep 11, 2019 · 8 comments

Comments

@jackkoppa
Copy link

commented Sep 11, 2019

  • I have searched through existing issues
  • I have read through docs
  • I have read FAQ

Info

  • Platform: macOS
  • Vetur version: 0.22.2
  • VS Code version: 1.38.0

Problem

Feature Request - add a third option for templateInterpolationService, ts-only

As mentioned in #1246, the templateInterpolationService works well in a lot of cases, but will quite regularly throw errors for JavaScript components, because without proper attention paid to adding type annotations to a component using lang="ts", it's very unlikely that a JS Vue component will be able to be parsed correctly.

So, in JS components, there are a lot of errors that end up looking like this, when setting "vetur.experimental.templateInterpolationService": true

Screen Shot 2019-09-11 at 7 37 39 AM

In our case, and potentially other repos that are attempting to get TS benefits in a codebase that was originally JS-only, some components are TS, while many remain JS.

It seems likely that only TS components will see useful benefits from templateInterpolationService,
so the proposal is to add a third option to the settings, ts-only

Changing these lines to something like

"vetur.experimental.templateInterpolationService": {
	"type": ["boolean", "string"],
	"default": false,
	"description": "Enable template interpolation service that offers diagnostics / hover / definition / references. Use 'ts-only' to enable only for TypeScript components"
}

I'm sure the above can be done much better, hopefully with enum, while still maintaining backwards compatibility (default still false, still allowing true & applying to all files).

I'm very happy to work on this change, if it's an acceptable request & the use case (JS & TS components in the same project, or at least the same workspace) is common enough. I don't have any significant extension development experience, so any other recommendations on how to structure the config option are welcome.

Reproducible Case

(happy to add a modified Veturpack with both JS & TS components, if that's useful)

@jackkoppa jackkoppa changed the title Allow applying `templateInterpolationService` only to `lang="ts"` components Allow applying templateInterpolationService only to lang="ts" components Sep 11, 2019
@jackkoppa

This comment has been minimized.

Copy link
Author

commented Sep 11, 2019

Two notes I forgot:

  1. The other option is of course to switch the meaning of templateInterpolationService: true to be "only on TS components". For an experimental feature, perhaps that "breaking-ish" change isn't an issue, and that certainly feels like the right behavior to me. As nice as it would be to get bug reports for JS files, it seems really unlikely that Vue 2.x JS components will ever get to the point where they can be reliably typed; i.e. it feels like we'll always need some of the computed prop annotations, or Vuex will always be too tricky, for JS component interpolation.

  2. I wanted to take the opportunity to say thank you so much for this plugin, and for the templateInterpolationService feature in particular. It's been astounding how well it's worked for our TypeScript components - not a single incorrect error thus far - and it's completely eliminated my TSX envy. Our biggest frustration with TypeScript in Vue has been "even if we type the component completely properly, and we get to use backend-generated interfaces to help with typing responses, accessing things in the template is still completely unchecked"
    And this feature has totally eliminated that concern. It's extremely impressive, and extremely useful - I don't think I got a chance to shout it out here, but I hope everyone using TS-in-Vue is enabling it. Please let me know if there's any specific testing or tasks you need with the template interpolation; it's been a serious boon 😀

@jackkoppa

This comment has been minimized.

Copy link
Author

commented Sep 19, 2019

Hi @ktsn or @octref - do you happen to have an opinion on whether this feature would be acceptable? I'd love to give it a shot if so, but only if the work is deemed worthwhile/in-line with the extension's goals

@ktsn

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

Thank you for your suggestion.

I prefer making template diagnostics available only lang=ts without adding a new config as template diagnostics would not make sense in JS.
Note that we should not disable other template interpolation service such as completion, hover info etc. just like how it works on JS file. We should only disable template diagnostics if it's not lang=ts

@ktsn

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

Maybe we also can enable template diagnostics if a JS <script> block has // @ts-check directive. The idea is that making interpolation service behavior to be same as how corresponding <script> block behave.

jackkoppa pushed a commit to jackkoppa/vetur that referenced this issue Sep 22, 2019
Add method in Vue info service to check whether a given language exists in the current document
Use that method to apply template interpolation checks only when TypeScript is found
Resolves vuejs#1427

TODO: Add specs if possible
jackkoppa added a commit to jackkoppa/vetur that referenced this issue Sep 22, 2019
Add method in Vue info service to check whether a given language exists in the current document
Use that method to apply template interpolation checks only when TypeScript is found
Resolves vuejs#1427

TODO: Add specs if possible
@jackkoppa

This comment has been minimized.

Copy link
Author

commented Sep 22, 2019

Thanks for the direction, @ktsn 👍

I haven't yet been able to figure out how we would recognize when // @ts-check is applied in #1439; any guidance on that would be very much appreciated.

@rchl

This comment has been minimized.

Copy link

commented Sep 30, 2019

I don't like the idea of disabling services for JS. They work fine, it is just that one needs to remember to add type annotations in computed properties, methods and couple more please. It would be a big loss if that didn't work.

Or am I misunderstanding what you are intending to disable?

@ktsn

This comment has been minimized.

Copy link
Member

commented Sep 30, 2019

I feel a bit weird that you expect template type check while <script> block doesn't (without @ts-check or checkJs in ts compiler). If you are ok with adding type annotation via JS docs, maybe you can just enable checkJs, can't you?

@rchl

This comment has been minimized.

Copy link

commented Sep 30, 2019

Yes, I have checkJs enabled and I have typechecking enabled in script block.
And now I understand your intention and I'm fine with template interpolation only be enabled when @ts-check or checkJs is enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.