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

New rule order-in-components #42

Merged
merged 11 commits into from
Jun 26, 2017
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ The `--fix` option on the command line automatically fixes problems reported by
| :white_check_mark: | [no-confusing-v-for-v-if](./docs/rules/no-confusing-v-for-v-if.md) | disallow confusing `v-for` and `v-if` on the same element. |
| | [no-duplicate-attributes](./docs/rules/no-duplicate-attributes.md) | disallow duplicate arguments. |
| :white_check_mark: | [no-textarea-mustache](./docs/rules/no-textarea-mustache.md) | disallow mustaches in `<textarea>`. |
| :white_check_mark: | [order-in-components](./docs/rules/order-in-components.md) | Keep order of properties in components |
| :white_check_mark: | [require-component-is](./docs/rules/require-component-is.md) | require `v-bind:is` of `<component>` elements. |
| :white_check_mark: | [require-v-for-key](./docs/rules/require-v-for-key.md) | require `v-bind:key` with `v-for` directives. |

Expand Down
87 changes: 87 additions & 0 deletions docs/rules/order-in-components.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Keep proper order of properties in your components (order-in-components)

This rule makes sure you keep declared order of properties in components.

## :book: Rule Details

Recommended order of properties is as follows:

1. Options / Misc (`name`, `delimiters`, `functional`, `model`)
2. Options / Assets (`components`, `directives`, `filters`)
3. Options / Composition (`parent`, `mixins`, `extends`, `provide`, `inject`)
4. `el`
5. `template`
6. `props`
7. `propsData`
8. `data`
9. `computed`
10. `watch`
11. `lifecycleHooks`
12. `methods`
13. `render`
14. `renderError`

Note that `lifecycleHooks` is not a regular property - it indicates the group of all lifecycle hooks just to simplify the configuration.

Examples of **incorrect** code for this rule:

```js

export default {
name: 'app',
data () {
return {
msg: 'Welcome to Your Vue.js App'
}
},
props: {
propA: Number,
},
}

```

Examples of **correct** code for this rule:

```js

export default {
name: 'app',
props: {
propA: Number,
},
data () {
return {
msg: 'Welcome to Your Vue.js App'
}
},
}

```

### Options

If you want you can change the order providing the optional configuration in your `.eslintrc` file. Setting responsible for the above order looks like this:

```
vue/order-in-components: [2, {
order: [
['name', 'delimiters', 'functional', 'model'],
['components', 'directives', 'filters'],
['parent', 'mixins', 'extends', 'provide', 'inject'],
'el',
'template',
'props',
'propsData',
'data',
'computed',
'watch',
'lifecycle_hooks',
'methods',
'render',
'renderError'
]
}]
```

If you want some of properties to be treated equally in order you can group them into arrays, like we did with `name`, `delimiters`, `funcitonal` and `model`.
1 change: 1 addition & 0 deletions lib/recommended-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports = {
"vue/no-invalid-v-text": "error",
"vue/no-parsing-error": "error",
"vue/no-textarea-mustache": "error",
"vue/order-in-components": "error",
"vue/require-component-is": "error",
"vue/require-v-for-key": "error",
"vue/v-bind-style": "off",
Expand Down
143 changes: 143 additions & 0 deletions lib/rules/order-in-components.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/**
* @fileoverview Keep order of properties in components
* @author Michał Sajnóg
*/
'use strict'

const defaultOrder = [
['name', 'delimiters', 'functional', 'model'],
['components', 'directives', 'filters'],
['parent', 'mixins', 'extends', 'provide', 'inject'],
'el',
'template',
'props',
'propsData',
'data',
'computed',
'watch',
'LIFECYCLE_HOOKS',
'methods',
'render',
'renderError'
]

const groups = {
LIFECYCLE_HOOKS: [
'beforeCreate',
'created',
'beforeMount',
'mounted',
'beforeUpdate',
'updated',
'activated',
'deactivated',
'beforeDestroy',
'destroyed'
]
}

function isComponentFile (path) {
return path.endsWith('.vue') || path.endsWith('.jsx')
}

function isVueComponent (node) {
const callee = node.callee

const isFullVueComponent = node.type === 'CallExpression' &&
callee.type === 'MemberExpression' &&
callee.object.type === 'Identifier' &&
callee.object.name === 'Vue' &&
callee.property.type === 'Identifier' &&
callee.property.name === 'component'

const isDestructedVueComponent = callee.type === 'Identifier' &&
callee.name === 'component'

return isFullVueComponent || isDestructedVueComponent
}

function isVueInstance (node) {
const callee = node.callee
return node.type === 'NewExpression' &&
callee.type === 'Identifier' &&
callee.name === 'Vue'
}

function getOrderMap (order) {
const orderMap = new Map()

order.forEach((property, i) => {
if (Array.isArray(property)) {
property.forEach(p => orderMap.set(p, i))
} else {
orderMap.set(property, i)
}
})

return orderMap
}

function checkOrder (propertiesNodes, orderMap, context) {
const properties = propertiesNodes.map(property => property.key)

properties.forEach((property, i) => {
const propertiesAbove = properties.slice(0, i)
const unorderedProperties = propertiesAbove
.filter(p => orderMap.get(p.name) > orderMap.get(property.name))
.sort((p1, p2) => orderMap.get(p1.name) > orderMap.get(p2.name))

const firstUnorderedProperty = unorderedProperties[0]

if (firstUnorderedProperty) {
const line = firstUnorderedProperty.loc.start.line
context.report({
node: property,
message: `The "${property.name}" property should be above the "${firstUnorderedProperty.name}" property on line ${line}.`
})
}
})
}

function create (context) {
const options = context.options[0] || {}
const order = options.order || defaultOrder
const filePath = context.getFilename()

const extendedOrder = order.map(property => groups[property] || property)
const orderMap = getOrderMap(extendedOrder)

return {
ExportDefaultDeclaration (node) {
// export default {} in .vue || .jsx
if (!isComponentFile(filePath)) return
checkOrder(node.declaration.properties, orderMap, context)
Copy link
Member

Choose a reason for hiding this comment

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

I guess that this line can cause TypeError if default exports without object literals (e.g. export default 777) exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

Very good point! Fixed :)

},
CallExpression (node) {
// Vue.component('xxx', {}) || component('xxx', {})
if (!isVueComponent(node)) return
checkOrder(node.arguments.slice(-1)[0].properties, orderMap, context)
Copy link
Member

Choose a reason for hiding this comment

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

I guess that this line can throw TypeError if component() with no argument exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, but I couldn't imagine such a component though. Fixed it anyway :)

Copy link
Member

Choose a reason for hiding this comment

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

Many people use ESLint in editor integrations. In that case, ESLint runs on every character typing, so TypeErrors mess development experience. 😉

},
NewExpression (node) {
// new Vue({})
if (!isVueInstance(node)) return
checkOrder(node.arguments[0].properties, orderMap, context)
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

}
}
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
create,
meta: {
docs: {
description: 'Keep order of properties in components',
category: 'Best Practices',
recommended: true
Copy link
Member

Choose a reason for hiding this comment

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

I think that it needs major version if we keep recommended: true.

Copy link
Member Author

@michalsnik michalsnik Jun 26, 2017

Choose a reason for hiding this comment

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

yeah, I was thinking the same. So maybe we'll wait with this until other PR's are merged, and then we'll bump to 4.0.0. If it will be quite stable - then we could finally release it without the beta tag.

},
fixable: null,
schema: []
}
}
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
"main": "lib/index.js",
"scripts": {
"start": "npm run test:simple -- --watch --growl",
"test:base": "mocha \"tests/lib/**/*.js\" \"tests/integrations/*.js\" --timeout 60000",
"test:base": "mocha \"tests/lib/**/*.js\"",
"test:simple": "npm run test:base -- --reporter nyan",
"test": "nyc npm run test:base",
"test": "nyc npm run test:base -- \"tests/integrations/*.js\" --timeout 60000",
"lint": "eslint .",
"pretest": "npm run lint",
"preversion": "npm run update && npm test",
Expand Down
Loading