Add new attributes order for dynamic and static props (#2068)

vuejs · Jan 12, 2023 · 0b816dc · 0b816dc
1 parent ad07deb
commit 0b816dc
Show file tree

Hide file tree

Showing 3 changed files with 293 additions and 4 deletions.
diff --git a/docs/rules/attributes-order.md b/docs/rules/attributes-order.md
@@ -35,7 +35,13 @@ This rule aims to enforce ordering of component attributes. The default order is
 - `OTHER_DIRECTIVES`
   e.g. 'v-custom-directive'
 - `OTHER_ATTR`
-  e.g. 'custom-prop="foo"', 'v-bind:prop="foo"', ':prop="foo"'
+  alias for `[ATTR_DYNAMIC, ATTR_STATIC, ATTR_SHORTHAND_BOOL]`:
+  - `ATTR_DYNAMIC`
+    e.g. 'v-bind:prop="foo"', ':prop="foo"'
+  - `ATTR_STATIC`
+    e.g. 'prop="foo"', 'custom-prop="foo"'
+  - `ATTR_SHORTHAND_BOOL`
+    e.g. 'boolean-prop'
 - `EVENTS`
   e.g. '@click="functionCall"', 'v-on="event"'
 - `CONTENT`

diff --git a/lib/rules/attributes-order.js b/lib/rules/attributes-order.js
@@ -20,6 +20,9 @@ const ATTRS = {
   TWO_WAY_BINDING: 'TWO_WAY_BINDING',
   OTHER_DIRECTIVES: 'OTHER_DIRECTIVES',
   OTHER_ATTR: 'OTHER_ATTR',
+  ATTR_STATIC: 'ATTR_STATIC',
+  ATTR_DYNAMIC: 'ATTR_DYNAMIC',
+  ATTR_SHORTHAND_BOOL: 'ATTR_SHORTHAND_BOOL',
   EVENTS: 'EVENTS',
   CONTENT: 'CONTENT'
 }
@@ -66,6 +69,15 @@ function isVBindObject(node) {
   return isVBind(node) && node.key.argument == null
 }
 
+/**
+ * Check whether the given attribute is a shorthand boolean like `selected`.
+ * @param {VAttribute | VDirective | undefined | null} node
+ * @returns { node is VAttribute }
+ */
+function isVShorthandBoolean(node) {
+  return isVAttribute(node) && !node.value
+}
+
 /**
  * @param {VAttribute | VDirective} attribute
  * @param {SourceCode} sourceCode
@@ -153,7 +165,13 @@ function getAttributeType(attribute) {
     case 'slot-scope':
       return ATTRS.SLOT
     default:
-      return ATTRS.OTHER_ATTR
+      if (isVBind(attribute)) {
+        return ATTRS.ATTR_DYNAMIC
+      }
+      if (isVShorthandBoolean(attribute)) {
+        return ATTRS.ATTR_SHORTHAND_BOOL
+      }
+      return ATTRS.ATTR_STATIC
   }
 }
 
@@ -191,6 +209,11 @@ function isAlphabetical(prevNode, currNode, sourceCode) {
  */
 function create(context) {
   const sourceCode = context.getSourceCode()
+  const otherAttrs = [
+    ATTRS.ATTR_DYNAMIC,
+    ATTRS.ATTR_STATIC,
+    ATTRS.ATTR_SHORTHAND_BOOL
+  ]
   let attributeOrder = [
     ATTRS.DEFINITION,
     ATTRS.LIST_RENDERING,
@@ -200,12 +223,36 @@ function create(context) {
     [ATTRS.UNIQUE, ATTRS.SLOT],
     ATTRS.TWO_WAY_BINDING,
     ATTRS.OTHER_DIRECTIVES,
-    ATTRS.OTHER_ATTR,
+    otherAttrs,
     ATTRS.EVENTS,
     ATTRS.CONTENT
   ]
   if (context.options[0] && context.options[0].order) {
-    attributeOrder = context.options[0].order
+    attributeOrder = [...context.options[0].order]
+
+    // check if `OTHER_ATTR` is valid
+    for (const item of attributeOrder.flat()) {
+      if (item === ATTRS.OTHER_ATTR) {
+        for (const attribute of attributeOrder.flat()) {
+          if (otherAttrs.includes(attribute)) {
+            throw new Error(
+              `Value "${ATTRS.OTHER_ATTR}" is not allowed with "${attribute}".`
+            )
+          }
+        }
+      }
+    }
+
+    // expand `OTHER_ATTR` alias
+    for (const [index, item] of attributeOrder.entries()) {
+      if (item === ATTRS.OTHER_ATTR) {
+        attributeOrder[index] = otherAttrs
+      } else if (Array.isArray(item) && item.includes(ATTRS.OTHER_ATTR)) {
+        const attributes = item.filter((i) => i !== ATTRS.OTHER_ATTR)
+        attributes.push(...otherAttrs)
+        attributeOrder[index] = attributes
+      }
+    }
   }
   const alphabetical = Boolean(
     context.options[0] && context.options[0].alphabetical

diff --git a/tests/lib/rules/attributes-order.js b/tests/lib/rules/attributes-order.js
@@ -483,6 +483,138 @@ tester.run('attributes-order', rule, {
         </div>
       </template>`,
       options: [{ order: ['LIST_RENDERING', 'CONDITIONALS'] }]
+    },
+
+    // https://github.com/vuejs/eslint-plugin-vue/issues/1728
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          :prop-bind="prop"
+          prop-one="prop"
+          prop-two="prop">
+        </div>
+      </template>`,
+      options: [
+        {
+          order: ['ATTR_DYNAMIC', 'ATTR_STATIC'],
+          alphabetical: false
+        }
+      ]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          v-model="a
+          :class="b"
+          :is="c"
+          prop-one="d"
+          class="e"
+          prop-two="f">
+        </div>
+      </template>`,
+      options: [
+        {
+          order: ['TWO_WAY_BINDING', 'ATTR_DYNAMIC', 'ATTR_STATIC'],
+          alphabetical: false
+        }
+      ]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          prop-one="a"
+          prop-three="b"
+          :prop-two="c">
+        </div>
+      </template>`,
+      options: [
+        {
+          order: [
+            'DEFINITION',
+            'LIST_RENDERING',
+            'CONDITIONALS',
+            'RENDER_MODIFIERS',
+            'GLOBAL',
+            ['UNIQUE', 'SLOT'],
+            'TWO_WAY_BINDING',
+            'OTHER_DIRECTIVES',
+            'ATTR_STATIC',
+            'ATTR_DYNAMIC',
+            'EVENTS',
+            'CONTENT'
+          ],
+          alphabetical: false
+        }
+      ]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          prop-one="a"
+          :prop-two="b"
+          prop-three="c">
+        </div>
+      </template>`,
+      options: [
+        {
+          order: [
+            'DEFINITION',
+            'LIST_RENDERING',
+            'CONDITIONALS',
+            'RENDER_MODIFIERS',
+            'GLOBAL',
+            ['UNIQUE', 'SLOT'],
+            'TWO_WAY_BINDING',
+            'OTHER_DIRECTIVES',
+            ['ATTR_STATIC', 'ATTR_DYNAMIC'],
+            'EVENTS',
+            'CONTENT'
+          ],
+          alphabetical: false
+        }
+      ]
+    },
+
+    // https://github.com/vuejs/eslint-plugin-vue/issues/1870
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          boolean-prop
+          prop-one="a"
+          prop-two="b"
+          :prop-three="c">
+        </div>
+      </template>`,
+      options: [
+        {
+          order: [
+            'DEFINITION',
+            'LIST_RENDERING',
+            'CONDITIONALS',
+            'RENDER_MODIFIERS',
+            'GLOBAL',
+            ['UNIQUE', 'SLOT'],
+            'TWO_WAY_BINDING',
+            'OTHER_DIRECTIVES',
+            'ATTR_SHORTHAND_BOOL',
+            'ATTR_STATIC',
+            'ATTR_DYNAMIC',
+            'EVENTS',
+            'CONTENT'
+          ],
+          alphabetical: false
+        }
+      ]
     }
   ],
 
@@ -1528,6 +1660,110 @@ tester.run('attributes-order', rule, {
           attr="foo"/>
       </template>`,
       errors: ['Attribute "@click" should go before "v-bind".']
+    },
+
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          v-bind:prop-one="a"
+          prop-two="b"
+          :prop-three="c"/>
+      </template>`,
+      options: [{ order: ['ATTR_STATIC', 'ATTR_DYNAMIC'] }],
+      output: `
+      <template>
+        <div
+          prop-two="b"
+          v-bind:prop-one="a"
+          :prop-three="c"/>
+      </template>`,
+      errors: ['Attribute "prop-two" should go before "v-bind:prop-one".']
+    },
+
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          :prop-one="a"
+          v-model="value"
+          prop-two="b"
+          :prop-three="c"
+          class="class"
+          boolean-prop/>
+      </template>`,
+      options: [
+        {
+          order: [
+            'LIST_RENDERING',
+            'CONDITIONALS',
+            'RENDER_MODIFIERS',
+            'TWO_WAY_BINDING',
+            'OTHER_DIRECTIVES',
+            'ATTR_DYNAMIC',
+            'ATTR_STATIC',
+            'ATTR_SHORTHAND_BOOL',
+            'EVENTS'
+          ]
+        }
+      ],
+      output: `
+      <template>
+        <div
+          v-model="value"
+          :prop-one="a"
+          :prop-three="c"
+          prop-two="b"
+          class="class"
+          boolean-prop/>
+      </template>`,
+      errors: [
+        'Attribute "v-model" should go before ":prop-one".',
+        'Attribute ":prop-three" should go before "prop-two".'
+      ]
+    },
+
+    {
+      filename: 'test.vue',
+      code: `
+      <template>
+        <div
+          :prop-one="a"
+          v-model="value"
+          boolean-prop
+          prop-two="b"
+          :prop-three="c"/>
+      </template>`,
+      options: [
+        {
+          order: [
+            'UNIQUE',
+            'LIST_RENDERING',
+            'CONDITIONALS',
+            'RENDER_MODIFIERS',
+            'GLOBAL',
+            'TWO_WAY_BINDING',
+            'OTHER_DIRECTIVES',
+            ['ATTR_STATIC', 'ATTR_DYNAMIC', 'ATTR_SHORTHAND_BOOL'],
+            'EVENTS',
+            'CONTENT',
+            'DEFINITION',
+            'SLOT'
+          ]
+        }
+      ],
+      output: `
+      <template>
+        <div
+          v-model="value"
+          :prop-one="a"
+          boolean-prop
+          prop-two="b"
+          :prop-three="c"/>
+      </template>`,
+      errors: ['Attribute "v-model" should go before ":prop-one".']
     }
   ]
 })