Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(warn): Warn on colon shorthand usage on directive (fix #10191) #10199

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

afontcu
Copy link
Member

@afontcu afontcu commented Jun 26, 2019

fix #10191

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

Other information:

<template>
  <div v-if="something">foo</div>  <!-- does not throw a warning -->
  <div :v-if="something">foo</div> <!-- throws a warning -->
</template>

There are several things I decided that could be improved, so any advice or suggestion is greatly appreciated :)

  • PR shows a warning for every bound attribute starting with v-. The reasoning is that "The v- prefix serves as a visual cue for identifying Vue-specific attributes in your templates." (source)
  • Do we need more test cases?
  • Is the warning message clear enough? If the attribute is not a valid Vue directive, should the message change?

Thanks!

src/compiler/parser/index.js Outdated Show resolved Hide resolved
test/unit/modules/compiler/parser.spec.js Outdated Show resolved Hide resolved
afontcu and others added 2 commits June 26, 2019 18:10
Co-Authored-By: Eduardo San Martin Morote <posva@users.noreply.github.com>
Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but now that I think about it, I think we could add the ="${value}" to the instead of part. Sorry I forgot 😆

@afontcu
Copy link
Member Author

afontcu commented Jun 26, 2019

np! Actually I was about to add that bit too 😂 but after not seeing it in the suggestion I restrained myself. Updated!

@Vannsl
Copy link

Vannsl commented Oct 3, 2019

Can't this PR be merged? :)

@posva posva changed the title Warn on colon shorthand usage on directive (fix #10191) feat(warn): Warn on colon shorthand usage on directive (fix #10191) Sep 4, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn if colon shorthand is used on v-if/v-html/etc.
5 participants