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

feat(compiler): add warning when directives other than v-for and v-if used on template tag #7857

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

Conversation

superMDguy
Copy link

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:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
According to the docs, the v-show directive doesn't support the <template> element. So, if a v-show is used on a template, it is essentially ignored. However, it currently does that without any warning. This pull request adds a warning when a v-show directive is used on a template tag.

Copy link
Member

@yyx990803 yyx990803 left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR! A few things to note:

  1. Actually, the only directives that make sense on <template> are v-if and v-for. So we can check and warn not only v-show, but any directives that are not v-if or v-for.

  2. This may not be the best place to have a big chunk of logic since it kinda breaks the readability of the function which is mostly branches. We should extract it into a separate function, something like:

if (process.env.NODE_ENV !== 'production') {
  checkInvalidTemplateDirs(el)
}

@superMDguy
Copy link
Author

I've made the changes. I was thinking other directives also wouldn't be allowed, but I wasn't sure if that was the case 100% of the time.

@superMDguy superMDguy changed the title feat(compiler): add warning when v-show used on template tag feat(compiler): add warning when directives other than v-for and v-if used on template tag Mar 22, 2018
@felixbuenemann
Copy link
Contributor

Shouldn't v-slot also be allowed?

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

Successfully merging this pull request may close these issues.

3 participants