Skip to content

Fix: no-invalid-template-root has false positive about empty templates (fixes #41) #44

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

Merged
merged 6 commits into from
Jun 27, 2017

Conversation

mysticatea
Copy link
Member

Fixes #41.

This PR removes the warnings about empty templates.

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

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

LGTM

{
filename: 'test.vue',
code: '<template>\n</template>',
errors: ['The template root requires exactly one element.']
code: '<template>\n</template>'

Choose a reason for hiding this comment

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

Empty template seems also bad since users can simply skip the template section. I would like to see exactly one element warning back.

Choose a reason for hiding this comment

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

I think template root should be empty only if it is an external template reference.

Related: vuejs/vetur#272

Copy link
Member Author

@mysticatea mysticatea Jun 24, 2017

Choose a reason for hiding this comment

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

@HerringtonDarkholme Could you know where the document about <template src="foo.html"> is? User guide seems to not say about <template src="foo.html">. Also HTML spec does not have the src attribute.

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@michalsnik michalsnik 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, let's see how it works in practice. Merge when you're ready.

code: '<template><slot></slot></template>',
errors: ["The template root disallows '<slot>' elements."]
code: '<template src="foo.html">abc</template>',
errors: ["The template root which has 'src' attribute requires empty."]
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about amending this message to be: The template root with 'src' attribute is required to be empty.

@@ -6,16 +6,21 @@ This rule checks whether every template root is valid.

This rule reports the template root if the following cases:
Copy link
Member

Choose a reason for hiding this comment

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

I know it's out of the scope of this task, but if you're already on it maybe you could change if to in. We can also do a separate PR with copy updates though, so whatever..

@michalsnik michalsnik added this to the Official release milestone Jun 26, 2017
@mysticatea
Copy link
Member Author

Thank you for corrections!

@michalsnik michalsnik merged commit 62c60d7 into master Jun 27, 2017
@michalsnik michalsnik deleted the issue41 branch June 27, 2017 10:55
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