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

Add vue/no-v-text rule #1605

Merged
merged 6 commits into from Aug 15, 2021
Merged

Add vue/no-v-text rule #1605

merged 6 commits into from Aug 15, 2021

Conversation

tyankatsu0105
Copy link
Contributor

close #1435

Add rule vue/no-v-text.

  • Fixable
    • But in the case of selfclose element, it can't be fixed.

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

Thank you for this PR!
I have some requests.

sidebarDepth: 0
title: vue/no-v-text
description: disallow use of v-text
since: v7.17.0
Copy link
Member

@ota-meshi ota-meshi Aug 13, 2021

Choose a reason for hiding this comment

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

"since" is set automatically at release. So for now remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done
6238491


- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

- However, when using selfClose to element with v-text like `<div v-text="foobar" />`, it can't be fixed.
Copy link
Member

Choose a reason for hiding this comment

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

The header part is automatically generated, so the autofix sentence may not come to the bottom in the future. Could you list it in another place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done
6238491

if (!vText.value) return
if (vText.value.type !== 'VExpressionContainer') return
if (!vText.value.expression) return
if (vText.value.expression.type !== 'Identifier') return
Copy link
Member

Choose a reason for hiding this comment

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

Why limit it to an Identifier? I think all expressions should be reported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's my misunderstood.
I don't know how to use v-text very much.
I'm going to fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can use utils.getDirective!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed.

node: vText.node,
loc: vText.loc,
message: "Don't use 'v-text'.",
fix(fixable) {
Copy link
Member

@ota-meshi ota-meshi Aug 13, 2021

Choose a reason for hiding this comment

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

I think autofix is very difficult. For example, converting the following expression to {{}} will result in a parse error in Vue2.

<div v-text="'</div>'"></div>

<div >{{'</div>'}}</div>

So if you want to implement autofix, we need to limit autofix by the content of the expression.
Or I think it's good not to implement autofix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I make sense of what there are some points that I have to think about.
Yeah. I agree with your mention point.
I will remove the fix function from this rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done
bc030be

lib/rules/no-v-text.js Outdated Show resolved Hide resolved
Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@ota-meshi ota-meshi merged commit 16a4c67 into vuejs:master Aug 15, 2021
@tyankatsu0105 tyankatsu0105 deleted the feat/no-v-text branch August 15, 2021 02:57
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/no-v-text
2 participants