Skip to content

Commit

Permalink
fix(compiler-dom): bail static stringfication on non-attr bindings
Browse files Browse the repository at this point in the history
fix #1128
  • Loading branch information
yyx990803 committed May 7, 2020
1 parent 2f69167 commit 304ab8c
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 26 deletions.
25 changes: 25 additions & 0 deletions packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,29 @@ describe('stringify static html', () => {
type: NodeTypes.VNODE_CALL
})
})

// #1128
test('should bail on non attribute bindings', () => {
const { ast } = compileWithStringify(
`<div><div><input indeterminate>${repeat(
`<span class="foo">foo</span>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
)}</div></div>`
)
expect(ast.hoists.length).toBe(1)
expect(ast.hoists[0]).toMatchObject({
type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION
})

const { ast: ast2 } = compileWithStringify(
`<div><div><input :indeterminate="true">${repeat(
`<span class="foo">foo</span>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
)}</div></div>`
)
expect(ast2.hoists.length).toBe(1)
expect(ast2.hoists[0]).toMatchObject({
type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION
})
})
})
55 changes: 40 additions & 15 deletions packages/compiler-dom/src/transforms/stringifyStatic.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* This module is Node-only.
*/
import {
NodeTypes,
ElementNode,
Expand All @@ -13,6 +16,7 @@ import {
isVoidTag,
isString,
isSymbol,
isKnownAttr,
escapeHtml,
toDisplayString,
normalizeClass,
Expand All @@ -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
Expand All @@ -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 `<p>`
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++) {
Expand All @@ -83,7 +108,7 @@ function shouldOptimize(node: ElementNode): boolean {
if (walk(child)) {
return true
}
if (bail) {
if (bailed) {
return false
}
}
Expand Down
52 changes: 41 additions & 11 deletions packages/shared/src/domAttrConfig.ts
Original file line number Diff line number Diff line change
@@ -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,` +
Expand Down Expand Up @@ -41,7 +45,9 @@ export const propsToAttrMap: Record<string, string | undefined> = {
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,` +
Expand All @@ -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`
)

0 comments on commit 304ab8c

Please sign in to comment.