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

fix(vue): resolve src attribute on the script block on Vue files #130

Merged
merged 6 commits into from
Jul 26, 2018

Conversation

ktsn
Copy link
Contributor

@ktsn ktsn commented Jul 25, 2018

This PR fixes #111 and vuejs/vue-cli#1104. Also probably fixes #85.

I've replaced vue-parser with vue-template-compiler which is Vue official package for parsing .vue file. Because vue-parser looks not maintained and it would be better to use officially provided lib for consistency.

I didn't include vue-template-compiler in dependencies because there is a known issue that mismatching with user-installed vue version.
The users should not feel inconvenience by this because vue-loader (official webpack loader for .vue file) enforces the users to install vue-template-compiler due to this issue. That means if the users want to use vue option of this plugin, they already install vue-template-compiler by themselves.

@johnnyreilly
Copy link
Member

johnnyreilly commented Jul 25, 2018

Thanks for the PR @ktsn! Looks initially good to me.

@prograhammer @CKGrafico @wonderful-panda I believe you're Vue users and have worked on the Vue support in the plugin. Would you be able to look at this PR and let us know what you think?

cc @yyx990803

@CKGrafico
Copy link
Contributor

Looks really nice!

@wonderful-panda
Copy link
Contributor

LGTM

@yyx990803
Copy link

👍

@johnnyreilly
Copy link
Member

Okay - I think we're good to merge! @ktsn can you comment on whether the vue-template-compiler types are directly consumable or not?

@ktsn
Copy link
Contributor Author

ktsn commented Jul 26, 2018

Thanks for taking a look at this PR 🙏

@johnnyreilly The current vue-template-compiler does not include the types but it would be included and we will be able to consume it directly in the future since there is a PR already (maybe vue-template-compiler v2.6.0?).

@johnnyreilly
Copy link
Member

Awesome! Then let's go with this for now and if that PR gets merged we can switch to consuming from there later. Thanks for your work!

@johnnyreilly johnnyreilly merged commit 6a83c7d into TypeStrong:master Jul 26, 2018
@ktsn ktsn deleted the fix-vue-src branch July 26, 2018 05:24
@johnnyreilly
Copy link
Member

Merged - this will be published once a new token has been generated for publishing. See #132

@nobelhuang
Copy link

nobelhuang commented Aug 7, 2018

I'm not pretty sure, but seems this is the change that will somewhat cause tslint to complain if no-consecutive-blank-lines rule is used when we try to compile those vue files written in TS/TSX. I guess it is just because vue-template-compiler didn't remove those blank lines after extracting ts part from SFC, then we got this error:

no-consecutive-blank-lines: Consecutive blank lines are forbidden

@ktsn
Copy link
Contributor Author

ktsn commented Aug 8, 2018

We should not trim blank line because reported diagnostics will point to wrong location if we do that.

I remember vue-template-compiler pads the blank lines with // to avoid such lint errors (almost the same approach with vue-parser) but it looks applicable only when <script> block lang is JavaScript.

I will make a PR to fix the issue later.

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

Successfully merging this pull request may close these issues.

[vue] src attribute on script block is not resolved (vue) TSLint is running on empty script tags
6 participants