-
-
Notifications
You must be signed in to change notification settings - Fork 681
New Rule vue/sort-keys #997
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
686e59f
to
ea1a563
Compare
ea1a563
to
49b4486
Compare
@@ -2,7 +2,7 @@ | |||
/coverage | |||
/node_modules | |||
/tests/fixtures | |||
/tests/integrations/*/node_modules | |||
/tests/integrations/eslint-plugin-import |
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.
EsLint 6.7.0 changes eslintignore parsing. The previous one seems to have ignored the whole folder which I'm now explicitly doing. Otherwise, you get errors about eslint-plugin-ignore not being installed when running npm run lint
after a clean install.
Thank you for this PR. Could you change to ignore the following Vue component options?
|
Hi, My goal was to make sure all the props, computed, data, methods, etc are in alphabetical order while allowing the component order to be set by That would make this example invalid
it would have to have default before type
The main goal, however, was to make this invalid:
and require it to have
|
In that case, what things would be enforced by this rule? Can you give an example of something that you think would be invalid (while also ignoring all the things you mentioned above)? |
I think rule #541 will report an error with the following code: props: {
foo: {
default: 'foo',
type: String // The `type` property should come before the `default` property.
},
} The following code is valid: props: {
foo: {
type: String,
default: 'foo'
},
} The following code does not report an error with rule #541. props: {
foo: { },
bar: { },
} |
Got it, I'll work on that change. |
@ota-meshi I think the latest commit addresses your comments. I made what to ignore configurable in case people don't want to use any of the new rules or want to force alphabetical order until those are ready. |
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.
Thank you for changing this PR:smile:
I have some requests.
lib/rules/sort-keys.js
Outdated
const reportErrors = (isVue) => { | ||
if (isVue) { | ||
errors = errors.filter((error) => { | ||
let filter = error.hasUpper |
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.
Vue component objects may not be top-level objects. Excluding it with hasUpper can cause false positives.
const obj = {
foo() {
Vue.component('my-component', {
name: 'app',
data() {}
})
}
}
lib/rules/sort-keys.js
Outdated
message: "Expected object keys to be in {{natual}}{{insensitive}}{{order}}ending order. '{{thisName}}' should be before '{{prevName}}'.", | ||
parentName: stack.upper && stack.upper.prevName, | ||
grandparentName: stack.upper && stack.upper.upper && stack.upper.upper.prevName, | ||
greatGrandparentName: stack.upper && stack.upper.upper && stack.upper.upper.upper && stack.upper.upper.upper.prevName, |
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.
It may false negatives if the objects are not chained.
export default {
computed: {
foo () {
return {
b,
a
}
}
}
}
docs/rules/sort-keys.md
Outdated
|
||
```json | ||
{ | ||
"sort-keys": ["error", "asc", {"caseSensitive": true, "natural": false, "minKeys": 2}] |
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.
Could you added properties be included in the example?
@ota-meshi Thanks for catching those issues. They are fixed now. Let me know if you see anymore. |
@ota-meshi Any further comments on this PR? |
@loren138 sorry, but I haven't checked it yet. |
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.
Thank you for the change! I'm sorry to have kept you waiting.
I found a typo. It looks good to me except for this.
Co-Authored-By: Yosuke Ota <[email protected]>
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.
Thank you!
This rule creates a version of sort-keys which is compatible with
order-in-components
.This addresses the issue raised in #286 as well fulfilling the request in #601.