fix(compiler-dom): properly stringify class/style bindings when hoist…

…ing static strings
vuejs · Feb 21, 2020 · 1b9b235 · 1b9b235
1 parent 189a0a3
commit 1b9b235
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 58 deletions.
diff --git a/packages/compiler-core/__tests__/transforms/transformElement.spec.ts b/packages/compiler-core/__tests__/transforms/transformElement.spec.ts
@@ -623,7 +623,7 @@ describe('compiler: element transform', () => {
 
   test(`props merging: style`, () => {
     const { node } = parseWithElementTransform(
-      `<div style="color: red" :style="{ color: 'red' }" />`,
+      `<div style="color: green" :style="{ color: 'red' }" />`,
       {
         nodeTransforms: [transformStyle, transformElement],
         directiveTransforms: {
@@ -646,7 +646,7 @@ describe('compiler: element transform', () => {
             elements: [
               {
                 type: NodeTypes.SIMPLE_EXPRESSION,
-                content: `_hoisted_1`,
+                content: `{"color":"green"}`,
                 isStatic: false
               },
               {

diff --git a/packages/compiler-dom/__tests__/__snapshots__/index.spec.ts.snap b/packages/compiler-dom/__tests__/__snapshots__/index.spec.ts.snap
@@ -2,9 +2,6 @@
 
 exports[`compile should contain standard transforms 1`] = `
 "const _Vue = Vue
-const { createVNode: _createVNode } = _Vue
-
-const _hoisted_1 = {}
 
 return function render(_ctx, _cache) {
   with (_ctx) {
@@ -14,7 +11,7 @@ return function render(_ctx, _cache) {
       _createVNode(\\"div\\", { textContent: text }, null, 8 /* PROPS */, [\\"textContent\\"]),
       _createVNode(\\"div\\", { innerHTML: html }, null, 8 /* PROPS */, [\\"innerHTML\\"]),
       _createVNode(\\"div\\", null, \\"test\\"),
-      _createVNode(\\"div\\", { style: _hoisted_1 }, \\"red\\"),
+      _createVNode(\\"div\\", { style: {\\"color\\":\\"red\\"} }, \\"red\\"),
       _createVNode(\\"div\\", { style: {color: 'green'} }, null, 4 /* STYLE */)
     ], 64 /* STABLE_FRAGMENT */))
   }

diff --git a/packages/compiler-dom/__tests__/index.spec.ts b/packages/compiler-dom/__tests__/index.spec.ts
@@ -5,7 +5,7 @@ describe('compile', () => {
     const { code } = compile(`<div v-text="text"></div>
         <div v-html="html"></div>
         <div v-cloak>test</div>
-        <div style="color=red">red</div>
+        <div style="color:red">red</div>
         <div :style="{color: 'green'}"></div>`)
 
     expect(code).toMatchSnapshot()

diff --git a/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts b/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts
@@ -77,8 +77,8 @@ describe('stringify static html', () => {
 
   test('serliazing constant bindings', () => {
     const { ast } = compileWithStringify(
-      `<div><div>${repeat(
-        `<span :class="'foo' + 'bar'">{{ 1 }} + {{ false }}</span>`,
+      `<div><div :style="{ color: 'red' }">${repeat(
+        `<span :class="[{ foo: true }, { bar: true }]">{{ 1 }} + {{ false }}</span>`,
         StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
       )}</div></div>`
     )
@@ -89,8 +89,8 @@ describe('stringify static html', () => {
       callee: CREATE_STATIC,
       arguments: [
         JSON.stringify(
-          `<div>${repeat(
-            `<span class="foobar">1 + false</span>`,
+          `<div style="color:red;">${repeat(
+            `<span class="foo bar">1 + false</span>`,
             StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
           )}</div>`
         )

diff --git a/packages/compiler-dom/__tests__/transforms/transformStyle.spec.ts b/packages/compiler-dom/__tests__/transforms/transformStyle.spec.ts
@@ -26,17 +26,8 @@ function transformWithStyleTransform(
 }
 
 describe('compiler: style transform', () => {
-  test('should transform into directive node and hoist value', () => {
-    const { root, node } = transformWithStyleTransform(
-      `<div style="color: red"/>`
-    )
-    expect(root.hoists).toMatchObject([
-      {
-        type: NodeTypes.SIMPLE_EXPRESSION,
-        content: `{"color":"red"}`,
-        isStatic: false
-      }
-    ])
+  test('should transform into directive node', () => {
+    const { node } = transformWithStyleTransform(`<div style="color: red"/>`)
     expect(node.props[0]).toMatchObject({
       type: NodeTypes.DIRECTIVE,
       name: `bind`,
@@ -47,7 +38,7 @@ describe('compiler: style transform', () => {
       },
       exp: {
         type: NodeTypes.SIMPLE_EXPRESSION,
-        content: `_hoisted_1`,
+        content: `{"color":"red"}`,
         isStatic: false
       }
     })
@@ -71,7 +62,7 @@ describe('compiler: style transform', () => {
           },
           value: {
             type: NodeTypes.SIMPLE_EXPRESSION,
-            content: `_hoisted_1`,
+            content: `{"color":"red"}`,
             isStatic: false
           }
         }

diff --git a/packages/compiler-dom/src/transforms/stringifyStatic.ts b/packages/compiler-dom/src/transforms/stringifyStatic.ts
@@ -14,7 +14,10 @@ import {
   isString,
   isSymbol,
   escapeHtml,
-  toDisplayString
+  toDisplayString,
+  normalizeClass,
+  normalizeStyle,
+  stringifyStyle
 } from '@vue/shared'
 
 // Turn eligible hoisted static trees into stringied static nodes, e.g.
@@ -84,8 +87,15 @@ function stringifyElement(
       }
     } else if (p.type === NodeTypes.DIRECTIVE && p.name === 'bind') {
       // constant v-bind, e.g. :foo="1"
+      let evaluated = evaluateConstant(p.exp as SimpleExpressionNode)
+      const arg = p.arg && (p.arg as SimpleExpressionNode).content
+      if (arg === 'class') {
+        evaluated = normalizeClass(evaluated)
+      } else if (arg === 'style') {
+        evaluated = stringifyStyle(normalizeStyle(evaluated))
+      }
       res += ` ${(p.arg as SimpleExpressionNode).content}="${escapeHtml(
-        evaluateConstant(p.exp as ExpressionNode)
+        evaluated
       )}"`
     }
   }
@@ -151,7 +161,7 @@ function evaluateConstant(exp: ExpressionNode): string {
       if (c.type === NodeTypes.TEXT) {
         res += c.content
       } else if (c.type === NodeTypes.INTERPOLATION) {
-        res += evaluateConstant(c.content)
+        res += toDisplayString(evaluateConstant(c.content))
       } else {
         res += evaluateConstant(c)
       }

diff --git a/packages/compiler-dom/src/transforms/transformStyle.ts b/packages/compiler-dom/src/transforms/transformStyle.ts
@@ -17,12 +17,11 @@ export const transformStyle: NodeTransform = (node, context) => {
     node.props.forEach((p, i) => {
       if (p.type === NodeTypes.ATTRIBUTE && p.name === 'style' && p.value) {
         // replace p with an expression node
-        const exp = context.hoist(parseInlineCSS(p.value.content, p.loc))
         node.props[i] = {
           type: NodeTypes.DIRECTIVE,
           name: `bind`,
           arg: createSimpleExpression(`style`, true, p.loc),
-          exp,
+          exp: parseInlineCSS(p.value.content, p.loc),
           modifiers: [],
           loc: p.loc
         }
@@ -45,5 +44,5 @@ function parseInlineCSS(
       tmp.length > 1 && (res[tmp[0].trim()] = tmp[1].trim())
     }
   })
-  return createSimpleExpression(JSON.stringify(res), false, loc)
+  return createSimpleExpression(JSON.stringify(res), false, loc, true)
 }
diff --git a/packages/compiler-ssr/__tests__/ssrElement.spec.ts b/packages/compiler-ssr/__tests__/ssrElement.spec.ts
@@ -101,7 +101,7 @@ describe('ssr: element', () => {
       expect(
         getCompiledString(`<div style="color:red;" :style="bar"></div>`)
       ).toMatchInlineSnapshot(
-        `"\`<div style=\\"\${_ssrRenderStyle([_hoisted_1, _ctx.bar])}\\"></div>\`"`
+        `"\`<div style=\\"\${_ssrRenderStyle([{\\"color\\":\\"red\\"}, _ctx.bar])}\\"></div>\`"`
       )
     })
 
@@ -184,7 +184,7 @@ describe('ssr: element', () => {
         )
       ).toMatchInlineSnapshot(`
         "\`<div\${_ssrRenderAttrs(_mergeProps({
-            style: [_hoisted_1, _ctx.b]
+            style: [{\\"color\\":\\"red\\"}, _ctx.b]
           }, _ctx.obj))}></div>\`"
       `)
     })

diff --git a/packages/compiler-ssr/__tests__/ssrVShow.spec.ts b/packages/compiler-ssr/__tests__/ssrVShow.spec.ts
@@ -16,11 +16,9 @@ describe('ssr: v-show', () => {
       .toMatchInlineSnapshot(`
       "const { ssrRenderStyle: _ssrRenderStyle } = require(\\"@vue/server-renderer\\")
 
-      const _hoisted_1 = {\\"color\\":\\"red\\"}
-
       return function ssrRender(_ctx, _push, _parent) {
         _push(\`<div style=\\"\${_ssrRenderStyle([
-          _hoisted_1,
+          {\\"color\\":\\"red\\"},
           (_ctx.foo) ? null : { display: \\"none\\" }
         ])}\\"></div>\`)
       }"
@@ -48,11 +46,9 @@ describe('ssr: v-show', () => {
     ).toMatchInlineSnapshot(`
       "const { ssrRenderStyle: _ssrRenderStyle } = require(\\"@vue/server-renderer\\")
 
-      const _hoisted_1 = {\\"color\\":\\"red\\"}
-
       return function ssrRender(_ctx, _push, _parent) {
         _push(\`<div style=\\"\${_ssrRenderStyle([
-          _hoisted_1,
+          {\\"color\\":\\"red\\"},
           { fontSize: 14 },
           (_ctx.foo) ? null : { display: \\"none\\" }
         ])}\\"></div>\`)
@@ -69,12 +65,10 @@ describe('ssr: v-show', () => {
       "const { mergeProps: _mergeProps } = require(\\"vue\\")
       const { ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
 
-      const _hoisted_1 = {\\"color\\":\\"red\\"}
-
       return function ssrRender(_ctx, _push, _parent) {
         _push(\`<div\${_ssrRenderAttrs(_mergeProps(_ctx.baz, {
           style: [
-            _hoisted_1,
+            {\\"color\\":\\"red\\"},
             { fontSize: 14 },
             (_ctx.foo) ? null : { display: \\"none\\" }
           ]

diff --git a/packages/server-renderer/src/helpers/ssrRenderAttrs.ts b/packages/server-renderer/src/helpers/ssrRenderAttrs.ts
@@ -1,11 +1,9 @@
-import { escapeHtml } from '@vue/shared'
+import { escapeHtml, stringifyStyle } from '@vue/shared'
 import {
   normalizeClass,
   normalizeStyle,
   propsToAttrMap,
-  hyphenate,
   isString,
-  isNoUnitNumericStyleProp,
   isOn,
   isSSRSafeAttrName,
   isBooleanAttr,
@@ -93,17 +91,5 @@ export function ssrRenderStyle(raw: unknown): string {
     return escapeHtml(raw)
   }
   const styles = normalizeStyle(raw)
-  let ret = ''
-  for (const key in styles) {
-    const value = styles[key]
-    const normalizedKey = key.indexOf(`--`) === 0 ? key : hyphenate(key)
-    if (
-      isString(value) ||
-      (typeof value === 'number' && isNoUnitNumericStyleProp(normalizedKey))
-    ) {
-      // only render valid values
-      ret += `${normalizedKey}:${value};`
-    }
-  }
-  return escapeHtml(ret)
+  return escapeHtml(stringifyStyle(styles))
 }
diff --git a/packages/shared/src/normalizeProp.ts b/packages/shared/src/normalizeProp.ts
@@ -1,4 +1,5 @@
-import { isArray, isString, isObject } from './'
+import { isArray, isString, isObject, hyphenate } from './'
+import { isNoUnitNumericStyleProp } from './domAttrConfig'
 
 export function normalizeStyle(
   value: unknown
@@ -19,6 +20,27 @@ export function normalizeStyle(
   }
 }
 
+export function stringifyStyle(
+  styles: Record<string, string | number> | undefined
+): string {
+  let ret = ''
+  if (!styles) {
+    return ret
+  }
+  for (const key in styles) {
+    const value = styles[key]
+    const normalizedKey = key.indexOf(`--`) === 0 ? key : hyphenate(key)
+    if (
+      isString(value) ||
+      (typeof value === 'number' && isNoUnitNumericStyleProp(normalizedKey))
+    ) {
+      // only render valid values
+      ret += `${normalizedKey}:${value};`
+    }
+  }
+  return ret
+}
+
 export function normalizeClass(value: unknown): string {
   let res = ''
   if (isString(value)) {