fix(runtime-dom): consistently remove boolean attributes for falsy va…

…lues (#4348)
vuejs · Aug 16, 2021 · 620a69b · 620a69b
1 parent f855ccb
commit 620a69b
Show file tree

Hide file tree

Showing 12 changed files with 70 additions and 22 deletions.
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> = {}