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: add prefer-use-template-ref rule #2554

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Thomasan1999
Copy link

resolves #2549

Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! 🙂

I have a few comments.

lib/rules/prefer-use-template-ref.js Outdated Show resolved Hide resolved
tests/lib/rules/prefer-use-template-ref.js Show resolved Hide resolved
tests/lib/rules/prefer-use-template-ref.js Outdated Show resolved Hide resolved
docs/rules/prefer-use-template-ref.md Outdated Show resolved Hide resolved
docs/rules/prefer-use-template-ref.md Outdated Show resolved Hide resolved
docs/rules/prefer-use-template-ref.md Outdated Show resolved Hide resolved
lib/rules/prefer-use-template-ref.js Outdated Show resolved Hide resolved
tests/lib/rules/prefer-use-template-ref.js Show resolved Hide resolved
<div ref="root" />
</template>

<script>
Copy link
Member

Choose a reason for hiding this comment

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

If it is a <script setup> test, the setup attribute is required.

Suggested change
<script>
<script setup>

docs/rules/prefer-use-template-ref.md Show resolved Hide resolved
@maxfrees

This comment was marked as spam.

@Thomasan1999
Copy link
Author

any update

@maxfrees I will split it to 2 PRs as @ota-meshi suggested. Sorry for the delay, I was not able to get to it recently, will try to finish at least the first part this weekend, most of it is already done.

@Thomasan1999
Copy link
Author

@FloEdelmann @ota-meshi Implemented all of your feedback. 🙂

data() {
return {
label: ref(),
label: ref(this.defaultLabel),
labelElem: ref(),
Copy link
Member

Choose a reason for hiding this comment

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

Are refs actually supported by Vue in Option API's data like this?

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't break but Vue acts like the ref isn't there at all, it will simply unwrap the value. I wanted to prove that it works even in such a case, should I remove the test?

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.

Rule proposal: vue/prefer-use-template-ref
4 participants