fix(runtime-dom): consistently remove boolean attributes for falsy values (#4348)

skirtles-code · web-flow · commit 620a69b871a0 · 2021-08-16T18:18:36.000-04:00
diff --git a/packages/compiler-ssr/__tests__/ssrElement.spec.ts b/packages/compiler-ssr/__tests__/ssrElement.spec.ts
@@ -177,7 +177,7 @@ describe('ssr: element', () => {
       expect(getCompiledString(`<input type="checkbox" :checked="checked">`))
         .toMatchInlineSnapshot(`
         "\`<input type=\\"checkbox\\"\${
-            (_ctx.checked) ? \\" checked\\" : \\"\\"
+            (_ssrIncludeBooleanAttr(_ctx.checked)) ? \\" checked\\" : \\"\\"
           }>\`"
       `)
     })
diff --git a/packages/compiler-ssr/__tests__/ssrVModel.spec.ts b/packages/compiler-ssr/__tests__/ssrVModel.spec.ts
@@ -37,13 +37,13 @@ describe('ssr: v-model', () => {
     expect(
       compileWithWrapper(`<input type="radio" value="foo" v-model="bar">`).code
     ).toMatchInlineSnapshot(`
-      "const { ssrLooseEqual: _ssrLooseEqual, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
+      "const { ssrLooseEqual: _ssrLooseEqual, ssrIncludeBooleanAttr: _ssrIncludeBooleanAttr, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
 
       return function ssrRender(_ctx, _push, _parent, _attrs) {
         _push(\`<div\${
           _ssrRenderAttrs(_attrs)
         }><input type=\\"radio\\" value=\\"foo\\"\${
-          (_ssrLooseEqual(_ctx.bar, \\"foo\\")) ? \\" checked\\" : \\"\\"
+          (_ssrIncludeBooleanAttr(_ssrLooseEqual(_ctx.bar, \\"foo\\"))) ? \\" checked\\" : \\"\\"
         }></div>\`)
       }"
     `)
@@ -52,15 +52,15 @@ describe('ssr: v-model', () => {
   test('<input type="checkbox">', () => {
     expect(compileWithWrapper(`<input type="checkbox" v-model="bar">`).code)
       .toMatchInlineSnapshot(`
-      "const { ssrLooseContain: _ssrLooseContain, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
+      "const { ssrLooseContain: _ssrLooseContain, ssrIncludeBooleanAttr: _ssrIncludeBooleanAttr, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
 
       return function ssrRender(_ctx, _push, _parent, _attrs) {
         _push(\`<div\${
           _ssrRenderAttrs(_attrs)
         }><input type=\\"checkbox\\"\${
-          ((Array.isArray(_ctx.bar))
+          (_ssrIncludeBooleanAttr((Array.isArray(_ctx.bar))
             ? _ssrLooseContain(_ctx.bar, null)
-            : _ctx.bar) ? \\" checked\\" : \\"\\"
+            : _ctx.bar)) ? \\" checked\\" : \\"\\"
         }></div>\`)
       }"
     `)
@@ -69,15 +69,15 @@ describe('ssr: v-model', () => {
       compileWithWrapper(`<input type="checkbox" value="foo" v-model="bar">`)
         .code
     ).toMatchInlineSnapshot(`
-      "const { ssrLooseContain: _ssrLooseContain, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
+      "const { ssrLooseContain: _ssrLooseContain, ssrIncludeBooleanAttr: _ssrIncludeBooleanAttr, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
 
       return function ssrRender(_ctx, _push, _parent, _attrs) {
         _push(\`<div\${
           _ssrRenderAttrs(_attrs)
         }><input type=\\"checkbox\\" value=\\"foo\\"\${
-          ((Array.isArray(_ctx.bar))
+          (_ssrIncludeBooleanAttr((Array.isArray(_ctx.bar))
             ? _ssrLooseContain(_ctx.bar, \\"foo\\")
-            : _ctx.bar) ? \\" checked\\" : \\"\\"
+            : _ctx.bar)) ? \\" checked\\" : \\"\\"
         }></div>\`)
       }"
     `)
@@ -87,13 +87,13 @@ describe('ssr: v-model', () => {
         `<input type="checkbox" :true-value="foo" :false-value="bar" v-model="baz">`
       ).code
     ).toMatchInlineSnapshot(`
-      "const { ssrLooseEqual: _ssrLooseEqual, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
+      "const { ssrLooseEqual: _ssrLooseEqual, ssrIncludeBooleanAttr: _ssrIncludeBooleanAttr, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
 
       return function ssrRender(_ctx, _push, _parent, _attrs) {
         _push(\`<div\${
           _ssrRenderAttrs(_attrs)
         }><input type=\\"checkbox\\"\${
-          (_ssrLooseEqual(_ctx.baz, _ctx.foo)) ? \\" checked\\" : \\"\\"
+          (_ssrIncludeBooleanAttr(_ssrLooseEqual(_ctx.baz, _ctx.foo))) ? \\" checked\\" : \\"\\"
         }></div>\`)
       }"
     `)
@@ -103,13 +103,13 @@ describe('ssr: v-model', () => {
         `<input type="checkbox" true-value="foo" false-value="bar" v-model="baz">`
       ).code
     ).toMatchInlineSnapshot(`
-      "const { ssrLooseEqual: _ssrLooseEqual, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
+      "const { ssrLooseEqual: _ssrLooseEqual, ssrIncludeBooleanAttr: _ssrIncludeBooleanAttr, ssrRenderAttrs: _ssrRenderAttrs } = require(\\"@vue/server-renderer\\")
 
       return function ssrRender(_ctx, _push, _parent, _attrs) {
         _push(\`<div\${
           _ssrRenderAttrs(_attrs)
         }><input type=\\"checkbox\\"\${
-          (_ssrLooseEqual(_ctx.baz, \\"foo\\")) ? \\" checked\\" : \\"\\"
+          (_ssrIncludeBooleanAttr(_ssrLooseEqual(_ctx.baz, \\"foo\\"))) ? \\" checked\\" : \\"\\"
         }></div>\`)
       }"
     `)
diff --git a/packages/compiler-ssr/src/runtimeHelpers.ts b/packages/compiler-ssr/src/runtimeHelpers.ts
@@ -10,6 +10,7 @@ export const SSR_RENDER_ATTRS = Symbol(`ssrRenderAttrs`)
 export const SSR_RENDER_ATTR = Symbol(`ssrRenderAttr`)
 export const SSR_RENDER_DYNAMIC_ATTR = Symbol(`ssrRenderDynamicAttr`)
 export const SSR_RENDER_LIST = Symbol(`ssrRenderList`)
+export const SSR_INCLUDE_BOOLEAN_ATTR = Symbol(`ssrIncludeBooleanAttr`)
 export const SSR_LOOSE_EQUAL = Symbol(`ssrLooseEqual`)
 export const SSR_LOOSE_CONTAIN = Symbol(`ssrLooseContain`)
 export const SSR_RENDER_DYNAMIC_MODEL = Symbol(`ssrRenderDynamicModel`)
@@ -28,6 +29,7 @@ export const ssrHelpers = {
   [SSR_RENDER_ATTR]: `ssrRenderAttr`,
   [SSR_RENDER_DYNAMIC_ATTR]: `ssrRenderDynamicAttr`,
   [SSR_RENDER_LIST]: `ssrRenderList`,
+  [SSR_INCLUDE_BOOLEAN_ATTR]: `ssrIncludeBooleanAttr`,
   [SSR_LOOSE_EQUAL]: `ssrLooseEqual`,
   [SSR_LOOSE_CONTAIN]: `ssrLooseContain`,
   [SSR_RENDER_DYNAMIC_MODEL]: `ssrRenderDynamicModel`,
diff --git a/packages/compiler-ssr/src/transforms/ssrTransformElement.ts b/packages/compiler-ssr/src/transforms/ssrTransformElement.ts
@@ -43,7 +43,8 @@ import {
   SSR_RENDER_DYNAMIC_ATTR,
   SSR_RENDER_ATTRS,
   SSR_INTERPOLATE,
-  SSR_GET_DYNAMIC_MODEL_PROPS
+  SSR_GET_DYNAMIC_MODEL_PROPS,
+  SSR_INCLUDE_BOOLEAN_ATTR
 } from '../runtimeHelpers'
 import { SSRTransformContext, processChildren } from '../ssrCodegenTransform'
 
@@ -237,7 +238,10 @@ export const ssrTransformElement: NodeTransform = (node, context) => {
                   if (isBooleanAttr(attrName)) {
                     openTag.push(
                       createConditionalExpression(
-                        value,
+                        createCallExpression(
+                          context.helper(SSR_INCLUDE_BOOLEAN_ATTR),
+                          [value]
+                        ),
                         createSimpleExpression(' ' + attrName, true),
                         createSimpleExpression('', true),
                         false /* no newline */
diff --git a/packages/runtime-dom/__tests__/patchAttrs.spec.ts b/packages/runtime-dom/__tests__/patchAttrs.spec.ts
@@ -23,6 +23,18 @@ describe('runtime-dom: attrs patching', () => {
     expect(el.getAttribute('readonly')).toBe('')
     patchProp(el, 'readonly', true, false)
     expect(el.getAttribute('readonly')).toBe(null)
+    patchProp(el, 'readonly', false, '')
+    expect(el.getAttribute('readonly')).toBe('')
+    patchProp(el, 'readonly', '', 0)
+    expect(el.getAttribute('readonly')).toBe(null)
+    patchProp(el, 'readonly', 0, '0')
+    expect(el.getAttribute('readonly')).toBe('')
+    patchProp(el, 'readonly', '0', false)
+    expect(el.getAttribute('readonly')).toBe(null)
+    patchProp(el, 'readonly', false, 1)
+    expect(el.getAttribute('readonly')).toBe('')
+    patchProp(el, 'readonly', 1, undefined)
+    expect(el.getAttribute('readonly')).toBe(null)
   })
 
   test('attributes', () => {
diff --git a/packages/runtime-dom/__tests__/patchProps.spec.ts b/packages/runtime-dom/__tests__/patchProps.spec.ts
@@ -43,6 +43,18 @@ describe('runtime-dom: props patching', () => {
     expect(el.multiple).toBe(true)
     patchProp(el, 'multiple', null, null)
     expect(el.multiple).toBe(false)
+    patchProp(el, 'multiple', null, true)
+    expect(el.multiple).toBe(true)
+    patchProp(el, 'multiple', null, 0)
+    expect(el.multiple).toBe(false)
+    patchProp(el, 'multiple', null, '0')
+    expect(el.multiple).toBe(true)
+    patchProp(el, 'multiple', null, false)
+    expect(el.multiple).toBe(false)
+    patchProp(el, 'multiple', null, 1)
+    expect(el.multiple).toBe(true)
+    patchProp(el, 'multiple', null, undefined)
+    expect(el.multiple).toBe(false)
   })
 
   test('innerHTML unmount prev children', () => {
diff --git a/packages/runtime-dom/src/modules/attrs.ts b/packages/runtime-dom/src/modules/attrs.ts
@@ -1,4 +1,9 @@
-import { isSpecialBooleanAttr, makeMap, NOOP } from '@vue/shared'
+import {
+  includeBooleanAttr,
+  isSpecialBooleanAttr,
+  makeMap,
+  NOOP
+} from '@vue/shared'
 import {
   compatUtils,
   ComponentInternalInstance,
@@ -28,7 +33,7 @@ export function patchAttr(
     // note we are only checking boolean attributes that don't have a
     // corresponding dom prop of the same name here.
     const isBoolean = isSpecialBooleanAttr(key)
-    if (value == null || (isBoolean && value === false)) {
+    if (value == null || (isBoolean && !includeBooleanAttr(value))) {
       el.removeAttribute(key)
     } else {
       el.setAttribute(key, isBoolean ? '' : value)
diff --git a/packages/runtime-dom/src/modules/props.ts b/packages/runtime-dom/src/modules/props.ts
@@ -3,6 +3,7 @@
 // This can come from explicit usage of v-html or innerHTML as a prop in render
 
 import { warn, DeprecationTypes, compatUtils } from '@vue/runtime-core'
+import { includeBooleanAttr } from '@vue/shared'
 
 // functions. The user is responsible for using them with only trusted content.
 export function patchDOMProp(
@@ -41,9 +42,9 @@ export function patchDOMProp(
 
   if (value === '' || value == null) {
     const type = typeof el[key]
-    if (value === '' && type === 'boolean') {
+    if (type === 'boolean') {
       // e.g. <select multiple> compiles to { multiple: '' }
-      el[key] = true
+      el[key] = includeBooleanAttr(value)
       return
     } else if (value == null && type === 'string') {
       // e.g. <div :id="null">
diff --git a/packages/server-renderer/__tests__/ssrRenderAttrs.spec.ts b/packages/server-renderer/__tests__/ssrRenderAttrs.spec.ts
@@ -50,9 +50,11 @@ describe('ssr: renderAttrs', () => {
     expect(
       ssrRenderAttrs({
         checked: true,
-        multiple: false
+        multiple: false,
+        readonly: 0,
+        disabled: ''
       })
-    ).toBe(` checked`) // boolean attr w/ false should be ignored
+    ).toBe(` checked disabled`) // boolean attr w/ false should be ignored
   })
 
   test('ignore falsy values', () => {
diff --git a/packages/server-renderer/src/helpers/ssrRenderAttrs.ts b/packages/server-renderer/src/helpers/ssrRenderAttrs.ts
@@ -7,6 +7,7 @@ import {
   isOn,
   isSSRSafeAttrName,
   isBooleanAttr,
+  includeBooleanAttr,
   makeMap
 } from '@vue/shared'
 
@@ -52,7 +53,7 @@ export function ssrRenderDynamicAttr(
       ? key // preserve raw name on custom elements
       : propsToAttrMap[key] || key.toLowerCase()
   if (isBooleanAttr(attrKey)) {
-    return value === false ? `` : ` ${attrKey}`
+    return includeBooleanAttr(value) ? ` ${attrKey}` : ``
   } else if (isSSRSafeAttrName(attrKey)) {
     return value === '' ? ` ${attrKey}` : ` ${attrKey}="${escapeHtml(value)}"`
   } else {
diff --git a/packages/server-renderer/src/index.ts b/packages/server-renderer/src/index.ts
@@ -27,6 +27,7 @@ export {
 export { ssrInterpolate } from './helpers/ssrInterpolate'
 export { ssrRenderList } from './helpers/ssrRenderList'
 export { ssrRenderSuspense } from './helpers/ssrRenderSuspense'
+export { includeBooleanAttr as ssrIncludeBooleanAttr } from '@vue/shared'
 
 // v-model helpers
 export {
diff --git a/packages/shared/src/domAttrConfig.ts b/packages/shared/src/domAttrConfig.ts
@@ -24,6 +24,14 @@ export const isBooleanAttr = /*#__PURE__*/ makeMap(
     `checked,muted,multiple,selected`
 )
 
+/**
+ * Boolean attributes should be included if the value is truthy or ''.
+ * e.g. <select multiple> compiles to { multiple: '' }
+ */
+export function includeBooleanAttr(value: unknown): boolean {
+  return !!value || value === ''
+}
+
 const unsafeAttrCharRE = /[>/="'\u0009\u000a\u000c\u0020]/
 const attrValidationCache: Record<string, boolean> = {}
 
