-
-
Notifications
You must be signed in to change notification settings - Fork 696
Add same order support on multiple types of attributes. #471
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
Conversation
Hey @tinymins Thanks for PR!
Can you provide more informations about this change?
- Is it a bug fix or improvement?
- What was the motivation?
- What problem does it solve?
- Is it related to any posted issue?
- Can you provide test cases that would fail before this change?
Thanks,
-
Is it a bug fix or improvement?
Improvement. -
What was the motivation?
In my project we want to sort attributes but we want some different kinds of attributes can have same priority. For example:<component id="a1" class="c1" :class="c2" style="color: black"></component>
In the code above, we want to force
idbeforeclassandstyle, but we don't care if:classis before or afterclass. -
What problem does it solve?
When lint sort attributes, some kinds of attributes can have same priority. -
Is it related to any posted issue?
Maybe not? I don't know if anyone else has the same problem. -
Can you provide test cases that would fail before this change?
Just like the example above, that case cannot be solved before this change, thus, we have to turn off this rule, because we do not want to force let:class(computed prop) before or afterclass(static prop).
What's more, I have an other improvement idea for this rule. If an attribute's type is not listed in attribute-order, it will be reported as wrong order if this attribute exist, no matter what position it is at. I think this is not right, attribute not be listed in order should be free at any position.
Alright @tinymins ! Thanks for clearing it out.
I think that your case was covered in #421, as we got rid of BINDING group there and treat bindings and other attrs equally. (It just waits for the major release).
If you however still want to ship support for grouping properties, I have to ask you to add tests covering the proposed enhancement - right here: https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/attributes-order.js
Although given the change in #421 I'm not sure it's still that relevant. What do you think?
c4f77f1 to
0115e46
Compare
Thanks, @michalsnik
I've read the file changes in #421, and I think my PR will get conflicted with #421. When will #421 got merged into master branch?
Hard to tell, maybe in week or two. Once we're done fixing current bugs and start preparing 5.0-beta
0115e46 to
01af72c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 GJ @tinymins
No description provided.