Skip to content

Add runOutsideVue option to vue/sort-keys #1866

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Add runOutsideVue option to vue/sort-keys #1866

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc60347
Add runOutsideVue option to vue/sort-keys
lukethompson Apr 23, 2022
97e971a
Sort vue data keys when runOutsideVue is false
lukethompson May 2, 2022
7b19499
Fix whitespace in test
lukethompson May 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/rules/sort-keys.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ export default {
"ignoreChildrenOf": ["model"],
"ignoreGrandchildrenOf": ["computed", "directives", "inject", "props", "watch"],
"minKeys": 2,
"natural": false
"natural": false,
"runOutsideVue": true,
}]
}
```
Expand All @@ -96,6 +97,7 @@ The 2nd option is an object which has 5 properties.
- `ignoreGrandchildrenOf` - an array of properties to ignore the grandchildren sort order. Default is `["computed", "directives", "inject", "props", "watch"]`
- `minKeys` - Specifies the minimum number of keys that an object should have in order for the object's unsorted keys to produce an error. Default is `2`, which means by default all objects with unsorted keys will result in lint errors.
- `natural` - if `true`, enforce properties to be in natural order. Default is `false`. Natural Order compares strings containing combination of letters and numbers in the way a human being would sort. It basically sorts numerically, instead of sorting alphabetically. So the number 10 comes after the number 3 in Natural Sorting.
- `runOutsideVue` - if `false`, this rule will ignore all keys outside of Vue. Default is `true`. Any child keys of a Vue instance are considered to be inside Vue.

While using this rule, you may disable the normal `sort-keys` rule. This rule will apply to plain js files as well as Vue component scripts.

Expand Down
6 changes: 5 additions & 1 deletion lib/rules/sort-keys.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ module.exports = {
const insensitive = options && options.caseSensitive === false
const minKeys = options && options.minKeys
const natural = options && options.natural
const onlyInsideVue = options && options.runOutsideVue === false
const isValidOrder =
isValidOrders[order + (insensitive ? 'I' : '') + (natural ? 'N' : '')]

Expand Down Expand Up @@ -247,7 +248,10 @@ module.exports = {
return
}
objectStack.vueState.currentProperty = node
if (objectStack.vueState.ignore) {
if (
objectStack.vueState.ignore ||
(onlyInsideVue && !objectStack.vueState.within)
) {
return
}
const prevName = objectStack.prevName
Expand Down
96 changes: 96 additions & 0 deletions tests/lib/rules/sort-keys.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,50 @@ ruleTester.run('sort-keys', rule, {
{
code: 'var obj = {a:1, _:2, b:3}',
options: ['desc', { natural: true, caseSensitive: false, minKeys: 4 }]
},

// runOutsideVue (false) should ignore unsorted keys outside of vue
{
code: 'var obj = { c:3, a:1, b:2}',
options: ['asc', { runOutsideVue: false }]
},
{
filename: 'test.js',
code: `
const { component } = Vue;
component('test', {
name: 'app',
data () {
return {
c: 3,
a: 1,
b: 2
}
Copy link
Member

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!
Why is this not reported? I think a, b, and c are Vue component properties and should be sorted.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the review.

I have pushed a change that will ensure objects returned by the data() method are sorted.

Just clarifying here, we only want to sort the keys of data() objects and we can leave objects defined within computed, watchers and methods unsorted?

If that is the case then this is all good.

If we want to also sort all new objects defined within a vue instance but not directly attached to the component maybe we should look towards implementing a new option that only sorts keys that are directly attached to the vue instance.

For example

...
computed: {
  // I would like the computed keys to be sorted.
  a () {
    // But I do not want to sort new objects defined within my computed.
     return {
       b: null,
       a: null,
     }
  },
}

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for late reply.

I think this rule should sort the keys of all objects that aren't reserved as Vue component definitions.
Therefore, I think the following b and a should also be sorted. Because they are inside the Vue component.

computed: {
  a () {
     return {
       b: null,
       a: null,
     }
  },
}

If you want to sort only the properties of Vue components (e.g. props, data, computed, watch, and methods), I think we can add another rule to exclude it from being checked by the vue/sort-keys rule.
I think it would be better to have a separate rule for sorting related to Vue components than vue/sort-keys rule, like vue/order-in-components rule.
https://eslint.vuejs.org/rules/order-in-components.html
What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Hello, sorry for the slow reply, been very busy with work lately.

This rule already accepts an ignoreGrandchildrenOf option that might be applicable here, iirc it only considers grandchildren keys of the same object, not newly created objects. Let me confirm this behaviour when I get a chance and maybe we could look towards making that ignore all grandchildren, not just grandchildren that are directly attached to the grandparent key.

Copy link
Member

Choose a reason for hiding this comment

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

The ignoreGrandchildrenOf option is an option to avoid conflicting with ordering conventions with Vue components.
If we need an option to ignore all grandchildren, I don't think that option should ignore the outside of the Vue component.

I think vue/sort-keys rule should have a role to enforce an order other than the Vue component ordering convention.
I think the order of Vue properties is the ordering convention of Vue components, so I think we need another rule.

I think runOutsideVue=false is just an option to ignore the outside of Vue. (However, I'm not sure if it's necessary for the user.)

I think it should be another rule if you want to enforce the order of Vue properties.
Where do you want to enforce the order?

Copy link
Author

Choose a reason for hiding this comment

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

@ota-meshi I was hoping to enforce order on all props, computed props, methods and watch child keys, eg.

export default {
  // These keys would not be enforced by this rule/option combination.
  name: 'ComponentName',

  props: {
    // These keys order would be enforced by this rule/option combination
    a: { type: String, default: null },
    b: { type: String, default: null },
  },
  
  computed: {
    c () {
      return {
        // These keys would not be enforced by this rule/option combination.
        e: null,
        d: null,
      }
    }
  },
}

I agree, I believe a new rule would be more appropriate for this. I have closed this PR for now and if I get a chance in the future I will submit a new PR, thanks.

}
})
`,
parserOptions: { ecmaVersion: 6 },
options: ['asc', { runOutsideVue: false }]
},
{
filename: 'test.js',
code: `
const { component } = Vue;
component('test', {
name: 'app',
computed: {
test () {
return {
c: 3,
a: 1,
b: 2
}
}
}
})
`,
parserOptions: { ecmaVersion: 6 },
options: ['asc', { runOutsideVue: false }]
}
],

Expand Down Expand Up @@ -1495,6 +1539,58 @@ ruleTester.run('sort-keys', rule, {
line: 7
}
]
},

// runOutsideVue (false)
{
filename: 'test.js',
code: `
const { component } = Vue;
component('test', {
name: 'app',
computed: {
b () {
return true
},
a () {
return true
}
}
})
`,
parserOptions,
options: ['asc', { runOutsideVue: false }],
errors: [
{
message:
"Expected object keys to be in ascending order. 'a' should be before 'b'.",
line: 9
}
]
},
{
filename: 'test.js',
code: `
const { component } = Vue;
component('test', {
name: 'app',
props: {
a: {
type: Boolean,
default: true
},
}
})
`,
parserOptions,
options: ['asc', { ignoreGrandchildrenOf: [], runOutsideVue: false }],
errors: [
{
message:
"Expected object keys to be in ascending order. 'default' should be before 'type'.",
line: 8
}
]
}
]
})