8000 Add new rule: vue/define-macros-order (#1855) by edikdeisling · Pull Request #1856 · vuejs/eslint-plugin-vue · GitHub
[go: up one dir, main page]

Skip to content

Add new rule: vue/define-macros-order (#1855) #1856

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 ou 8000 r terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 21, 2022

Conversation

edikdeisling
Copy link
Contributor

No description provided.

@FloEdelmann FloEdelmann linked an issue Apr 19, 2022 that may be closed by this pull request
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.

Thank you for this suggestion and implementation. I've only done a preliminary review for now.

@edikdeisling
Copy link
Contributor Author

There are a few thoughts:

  1. Maybe it would be better to have defineEmitsFirst: true rather than order: ['defineEmits', 'defineProps']? Or there are maybe some other define* that could come in?
  2. Do we need to give an ability to specify a new line between these statements? newLineBetween: true

@FloEdelmann
Copy link
Member

Maybe it would be better to have defineEmitsFirst: true rather than order: ['defineEmits', 'defineProps']? Or there are maybe some other define* that could come in?

I like order better, as it mimics the option from the existing vue/order-in-components rule.

Do we need to give an ability to specify a new line between these statements?

I don't think that should be part of this rule.

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.

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 wrote some comments.

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 213042c into vuejs:master Apr 21, 2022
@edikdeisling
Copy link
Contributor Author

@ota-meshi I found a bug :(
This plugin breaks code with <script> and <script setup> tags in one file.

I'll create PR soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FloEdelmann FloEdelmann FloEdelmann approved these changes

@ota-meshi ota-meshi ota-meshi approved these changes

Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vue/define-macros-order
3 participants
0