From 620a69b871a017dfe0ba81d380fd933d997c8a00 Mon Sep 17 00:00:00 2001 From: skirtle <65301168+skirtles-code@users.noreply.github.com> Date: Mon, 16 Aug 2021 23:18:36 +0100 Subject: [PATCH] fix(runtime-dom): consistently remove boolean attributes for falsy values (#4348) --- .../compiler-ssr/__tests__/ssrElement.spec.ts | 2 +- .../compiler-ssr/__tests__/ssrVModel.spec.ts | 24 +++++++++---------- packages/compiler-ssr/src/runtimeHelpers.ts | 2 ++ .../src/transforms/ssrTransformElement.ts | 8 +++++-- .../runtime-dom/__tests__/patchAttrs.spec.ts | 12 ++++++++++ .../runtime-dom/__tests__/patchProps.spec.ts | 12 ++++++++++ packages/runtime-dom/src/modules/attrs.ts | 9 +++++-- packages/runtime-dom/src/modules/props.ts | 5 ++-- .../__tests__/ssrRenderAttrs.spec.ts | 6 +++-- .../src/helpers/ssrRenderAttrs.ts | 3 ++- packages/server-renderer/src/index.ts | 1 + packages/shared/src/domAttrConfig.ts | 8 +++++++ 12 files changed, 70 insertions(+), 22 deletions(-) diff --git a/packages/compiler-ssr/__tests__/ssrElement.spec.ts b/packages/compiler-ssr/__tests__/ssrElement.spec.ts index d77b607a296..b7de9f69c31 100644 --- 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(``)) .toMatchInlineSnapshot(` "\`\`" `) }) diff --git a/packages/compiler-ssr/__tests__/ssrVModel.spec.ts b/packages/compiler-ssr/__tests__/ssrVModel.spec.ts index 21ce80a00bd..c6f57e0708c 100644 --- 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(``).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(\`\`) }" `) @@ -52,15 +52,15 @@ describe('ssr: v-model', () => { test('', () => { expect(compileWithWrapper(``).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(\`\`) }" `) @@ -69,15 +69,15 @@ describe('ssr: v-model', () => { compileWithWrapper(``) .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(\`\`) }" `) @@ -87,13 +87,13 @@ describe('ssr: v-model', () => { `` ).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(\`\`) }" `) @@ -103,13 +103,13 @@ describe('ssr: v-model', () => { `` ).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(\`\`) }" `) diff --git a/packages/compiler-ssr/src/runtimeHelpers.ts b/packages/compiler-ssr/src/runtimeHelpers.ts index 2aa93c0bf15..9be6c610a93 100644 --- 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 index 861446d562d..c9515d27baa 100644 --- 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 index ca48c10fea2..b78dd44c634 100644 --- 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 index 3d00e51ccfe..e1ee7928c84 100644 --- 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 index fa217800525..a80936345ed 100644 --- 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 index 36bb0c11ea3..aff6909f886 100644 --- 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. compiles to { multiple: '' } + */ +export function includeBooleanAttr(value: unknown): boolean { + return !!value || value === '' +} + const unsafeAttrCharRE = /[>/="'\u0009\u000a\u000c\u0020]/ const attrValidationCache: Record = {}