From 304ab8c99b954de4aa9ab6a5387116228345f544 Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 7 May 2020 10:32:13 -0400 Subject: [PATCH] fix(compiler-dom): bail static stringfication on non-attr bindings fix #1128 --- .../transforms/stringifyStatic.spec.ts | 25 +++++++++ .../src/transforms/stringifyStatic.ts | 55 ++++++++++++++----- packages/shared/src/domAttrConfig.ts | 52 ++++++++++++++---- 3 files changed, 106 insertions(+), 26 deletions(-) diff --git a/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts b/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts index 3dca19a0e8d..cf1c7e0a6d8 100644 --- a/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts +++ b/packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts @@ -168,4 +168,29 @@ describe('stringify static html', () => { type: NodeTypes.VNODE_CALL }) }) + + // #1128 + test('should bail on non attribute bindings', () => { + const { ast } = compileWithStringify( + `
${repeat( + `foo`, + StringifyThresholds.ELEMENT_WITH_BINDING_COUNT + )}
` + ) + expect(ast.hoists.length).toBe(1) + expect(ast.hoists[0]).toMatchObject({ + type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION + }) + + const { ast: ast2 } = compileWithStringify( + `
${repeat( + `foo`, + StringifyThresholds.ELEMENT_WITH_BINDING_COUNT + )}
` + ) + expect(ast2.hoists.length).toBe(1) + expect(ast2.hoists[0]).toMatchObject({ + type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION + }) + }) }) diff --git a/packages/compiler-dom/src/transforms/stringifyStatic.ts b/packages/compiler-dom/src/transforms/stringifyStatic.ts index f648257e489..3ed6cb2c045 100644 --- a/packages/compiler-dom/src/transforms/stringifyStatic.ts +++ b/packages/compiler-dom/src/transforms/stringifyStatic.ts @@ -1,3 +1,6 @@ +/** + * This module is Node-only. + */ import { NodeTypes, ElementNode, @@ -13,6 +16,7 @@ import { isVoidTag, isString, isSymbol, + isKnownAttr, escapeHtml, toDisplayString, normalizeClass, @@ -38,6 +42,11 @@ export const enum StringifyThresholds { NODE_COUNT = 20 } +const dataAriaRE = /^(data|aria)-/ +const isStringifiableAttr = (name: string) => { + return isKnownAttr(name) || dataAriaRE.test(name) +} + // Opt-in heuristics based on: // 1. number of elements with attributes > 5. // 2. OR: number of total nodes > 20 @@ -47,28 +56,44 @@ export const enum StringifyThresholds { function shouldOptimize(node: ElementNode): boolean { let bindingThreshold = StringifyThresholds.ELEMENT_WITH_BINDING_COUNT let nodeThreshold = StringifyThresholds.NODE_COUNT - let bail = false + + let bailed = false + const bail = () => { + bailed = true + return false + } // TODO: check for cases where using innerHTML will result in different // output compared to imperative node insertions. // probably only need to check for most common case // i.e. non-phrasing-content tags inside `

` function walk(node: ElementNode) { - // some transforms, e.g. `transformAssetUrls` in `@vue/compiler-sfc` may - // convert static attributes into a v-bind with a constnat expresion. - // Such constant bindings are eligible for hoisting but not for static - // stringification because they cannot be pre-evaluated. for (let i = 0; i < node.props.length; i++) { const p = node.props[i] - if ( - p.type === NodeTypes.DIRECTIVE && - p.name === 'bind' && - p.exp && - p.exp.type !== NodeTypes.COMPOUND_EXPRESSION && - p.exp.isRuntimeConstant - ) { - bail = true - return false + // bail on non-attr bindings + if (p.type === NodeTypes.ATTRIBUTE && !isStringifiableAttr(p.name)) { + return bail() + } + if (p.type === NodeTypes.DIRECTIVE && p.name === 'bind') { + // bail on non-attr bindings + if ( + p.arg && + (p.arg.type === NodeTypes.COMPOUND_EXPRESSION || + (p.arg.isStatic && !isStringifiableAttr(p.arg.content))) + ) { + return bail() + } + // some transforms, e.g. `transformAssetUrls` in `@vue/compiler-sfc` may + // convert static attributes into a v-bind with a constnat expresion. + // Such constant bindings are eligible for hoisting but not for static + // stringification because they cannot be pre-evaluated. + if ( + p.exp && + (p.exp.type === NodeTypes.COMPOUND_EXPRESSION || + p.exp.isRuntimeConstant) + ) { + return bail() + } } } for (let i = 0; i < node.children.length; i++) { @@ -83,7 +108,7 @@ function shouldOptimize(node: ElementNode): boolean { if (walk(child)) { return true } - if (bail) { + if (bailed) { return false } } diff --git a/packages/shared/src/domAttrConfig.ts b/packages/shared/src/domAttrConfig.ts index 5ae2ab7a2d9..689533dd92c 100644 --- a/packages/shared/src/domAttrConfig.ts +++ b/packages/shared/src/domAttrConfig.ts @@ -1,18 +1,22 @@ import { makeMap } from './makeMap' -// On the client we only need to offer special cases for boolean attributes that -// have different names from their corresponding dom properties: -// - itemscope -> N/A -// - allowfullscreen -> allowFullscreen -// - formnovalidate -> formNoValidate -// - ismap -> isMap -// - nomodule -> noModule -// - novalidate -> noValidate -// - readonly -> readOnly +/** + * On the client we only need to offer special cases for boolean attributes that + * have different names from their corresponding dom properties: + * - itemscope -> N/A + * - allowfullscreen -> allowFullscreen + * - formnovalidate -> formNoValidate + * - ismap -> isMap + * - nomodule -> noModule + * - novalidate -> noValidate + * - readonly -> readOnly + */ const specialBooleanAttrs = `itemscope,allowfullscreen,formnovalidate,ismap,nomodule,novalidate,readonly` export const isSpecialBooleanAttr = /*#__PURE__*/ makeMap(specialBooleanAttrs) -// The full list is needed during SSR to produce the correct initial markup. +/** + * The full list is needed during SSR to produce the correct initial markup. + */ export const isBooleanAttr = /*#__PURE__*/ makeMap( specialBooleanAttrs + `,async,autofocus,autoplay,controls,default,defer,disabled,hidden,` + @@ -41,7 +45,9 @@ export const propsToAttrMap: Record = { httpEquiv: 'http-equiv' } -// CSS properties that accept plain numbers +/** + * CSS properties that accept plain numbers + */ export const isNoUnitNumericStyleProp = /*#__PURE__*/ makeMap( `animation-iteration-count,border-image-outset,border-image-slice,` + `border-image-width,box-flex,box-flex-group,box-ordinal-group,column-count,` + @@ -53,3 +59,27 @@ export const isNoUnitNumericStyleProp = /*#__PURE__*/ makeMap( `fill-opacity,flood-opacity,stop-opacity,stroke-dasharray,stroke-dashoffset,` + `stroke-miterlimit,stroke-opacity,stroke-width` ) + +/** + * Known attributes, this is used for stringification of runtime static nodes + * so that we don't stringify bindings that cannot be set from HTML. + * Don't also forget to allow `data-*` and `aria-*`! + * Generated from https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes + */ +export const isKnownAttr = /*#__PURE__*/ makeMap( + `accept,accept-charset,accesskey,action,align,allow,alt,async,` + + `autocapitalize,autocomplete,autofocus,autoplay,background,bgcolor,` + + `border,buffered,capture,challenge,charset,checked,cite,class,code,` + + `codebase,color,cols,colspan,content,contenteditable,contextmenu,controls,` + + `coords,crossorigin,csp,data,datetime,decoding,default,defer,dir,dirname,` + + `disabled,download,draggable,dropzone,enctype,enterkeyhint,for,form,` + + `formaction,formenctype,formmethod,formnovalidate,formtarget,headers,` + + `height,hidden,high,href,hreflang,http-equiv,icon,id,importance,integrity,` + + `ismap,itemprop,keytype,kind,label,lang,language,loading,list,loop,low,` + + `manifest,max,maxlength,minlength,media,min,multiple,muted,name,novalidate,` + + `open,optimum,pattern,ping,placeholder,poster,preload,radiogroup,readonly,` + + `referrerpolicy,rel,required,reversed,rows,rowspan,sandbox,scope,scoped,` + + `selected,shape,size,sizes,slot,span,spellcheck,src,srcdoc,srclang,srcset,` + + `start,step,style,summary,tabindex,target,title,translate,type,usemap,` + + `value,width,wrap` +)