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: Add vue/no-static-inline-styles rule #843

Merged
merged 3 commits into from Dec 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/rules/README.md
Expand Up @@ -159,6 +159,7 @@ For example:
| [vue/no-empty-pattern](./no-empty-pattern.md) | disallow empty destructuring patterns | |
| [vue/no-reserved-component-names](./no-reserved-component-names.md) | disallow the use of reserved names in component definitions | |
| [vue/no-restricted-syntax](./no-restricted-syntax.md) | disallow specified syntax | |
| [vue/no-static-inline-styles](./no-static-inline-styles.md) | disallow static inline `style` attributes | |
| [vue/no-unsupported-features](./no-unsupported-features.md) | disallow unsupported Vue.js syntax on the specified version | :wrench: |
| [vue/object-curly-spacing](./object-curly-spacing.md) | enforce consistent spacing inside braces | :wrench: |
| [vue/require-direct-export](./require-direct-export.md) | require the component to be directly exported | |
Expand Down
69 changes: 69 additions & 0 deletions docs/rules/no-static-inline-styles.md
@@ -0,0 +1,69 @@
---
pageClass: rule-details
sidebarDepth: 0
title: vue/no-static-inline-styles
description: disallow static inline `style` attributes
---
# vue/no-static-inline-styles
> disallow static inline `style` attributes

## :book: Rule Details

This rule reports static inline `style` bindings and `style` attributes.
The styles reported in this rule mean that we recommend separating them into `<style>` tag.

<eslint-code-block :rules="{'vue/no-static-inline-styles': ['error']}">

```vue
<template>
<!-- ✓ GOOD -->
<div :style="styleObject"></div>
<div :style="{ backgroundImage: 'url('+img+')' }"></div>

<!-- ✗ BAD -->
<div style="color: pink;"></div>
<div :style="{ color: 'pink' }"></div>
<div :style="[ { color: 'pink' }, { 'font-size': '85%' } ]"></div>
<div :style="{ backgroundImage, color: 'pink' }"></div>
</template>
```

</eslint-code-block>

## :wrench: Options

```json
{
"vue/no-static-inline-styles": ["error", {
"allowBinding": false
}]
}
```

- allowBinding ... if `true`, allow binding static inline `style`. default `false`.

### `"allowBinding": true`

<eslint-code-block :rules="{'vue/no-static-inline-styles': ['error', {'allowBinding': true}]}">

```vue
<template>
<!-- ✓ GOOD -->
<div :style="{ transform: 'scale(0.5)' }"></div>
<div :style="[ { transform: 'scale(0.5)' }, { 'user-select': 'none' } ]"></div>

<!-- ✗ BAD -->
<div style="transform: scale(0.5);"></div>
</template>
```

</eslint-code-block>

## :books: Further reading

- [Guide - Binding Inline Styles](https://vuejs.org/v2/guide/class-and-style.html#Binding-Inline-Styles)

## :mag: Implementation

- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/no-static-inline-styles.js)
- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/no-static-inline-styles.js)
1 change: 1 addition & 0 deletions lib/index.js
Expand Up @@ -51,6 +51,7 @@ module.exports = {
'no-shared-component-data': require('./rules/no-shared-component-data'),
'no-side-effects-in-computed-properties': require('./rules/no-side-effects-in-computed-properties'),
'no-spaces-around-equal-signs-in-attribute': require('./rules/no-spaces-around-equal-signs-in-attribute'),
'no-static-inline-styles': require('./rules/no-static-inline-styles'),
'no-template-key': require('./rules/no-template-key'),
'no-template-shadow': require('./rules/no-template-shadow'),
'no-textarea-mustache': require('./rules/no-textarea-mustache'),
Expand Down
138 changes: 138 additions & 0 deletions lib/rules/no-static-inline-styles.js
@@ -0,0 +1,138 @@
/**
* @author Yosuke Ota
* See LICENSE file in root directory for full license.
*/
'use strict'

const utils = require('../utils')
module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'disallow static inline `style` attributes',
category: undefined,
url: 'https://eslint.vuejs.org/rules/no-static-inline-styles.html'
},
fixable: null,
schema: [
{
type: 'object',
properties: {
allowBinding: {
type: 'boolean'
}
},
additionalProperties: false
}
],
messages: {
forbiddenStaticInlineStyle: 'Static inline `style` are forbidden.',
forbiddenStyleAttr: '`style` attributes are forbidden.'
}
},
create (context) {
/**
* Checks whether if the given property node is a static value.
* @param {AssignmentProperty} prop property node to check
* @returns {boolean} `true` if the given property node is a static value.
*/
function isStaticValue (prop) {
return (
!prop.computed &&
prop.value.type === 'Literal' &&
(prop.key.type === 'Identifier' || prop.key.type === 'Literal')
)
}

/**
* Gets the static properties of a given expression node.
* - If `SpreadElement` or computed property exists, it gets only the static properties before it.
* `:style="{ color: 'red', display: 'flex', ...spread, width: '16px' }"`
* ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^
* - If non-static object exists, it gets only the static properties up to that object.
* `:style="[ { color: 'red' }, { display: 'flex', color, width: '16px' }, { height: '16px' } ]"`
* ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^
* - If all properties are static properties, it returns one root node.
* `:style="[ { color: 'red' }, { display: 'flex', width: '16px' } ]"`
* ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* @param {VAttribute} node `:style` node to check
* @returns {AssignmentProperty[] | [VAttribute]} the static properties.
*/
function getReportNodes (node) {
const { value } = node
if (!value) {
return []
}
const { expression } = value
if (!expression) {
return []
}

let elements
if (expression.type === 'ObjectExpression') {
elements = [expression]
} else if (expression.type === 'ArrayExpression') {
elements = expression.elements
} else {
return []
}
const staticProperties = []
for (const element of elements) {
if (!element) {
continue
}
if (element.type !== 'ObjectExpression') {
return staticProperties
}

let isAllStatic = true
for (const prop of element.properties) {
if (prop.type === 'SpreadElement' || prop.computed) {
// If `SpreadElement` or computed property exists, it gets only the static properties before it.
return staticProperties
}
if (isStaticValue(prop)) {
staticProperties.push(prop)
} else {
isAllStatic = false
}
}
if (!isAllStatic) {
// If non-static object exists, it gets only the static properties up to that object.
return staticProperties
}
}
// If all properties are static properties, it returns one root node.
return [node]
}

/**
* Reports if the value is static.
* @param {VAttribute} node `:style` node to check
*/
function verifyVBindStyle (node) {
for (const n of getReportNodes(node)) {
context.report({
node: n,
messageId: 'forbiddenStaticInlineStyle'
})
}
}

const visitor = {
"VAttribute[directive=false][key.name='style']" (node) {
context.report({
node,
messageId: 'forbiddenStyleAttr'
})
}
}
if (!context.options[0] || !context.options[0].allowBinding) {
visitor[
"VAttribute[directive=true][key.name.name='bind'][key.argument.name='style']"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be [key.name='bind'][key.argument='style'] ?

Copy link
Member

Choose a reason for hiding this comment

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

After #807, AST has been changed a bit to support dynamic argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah of course. Thank you for the explanation.

] = verifyVBindStyle
}

return utils.defineTemplateBodyVisitor(context, visitor)
}
}