Skip to content

Commit 304ab8c

Browse files
committed
fix(compiler-dom): bail static stringfication on non-attr bindings
fix #1128
1 parent 2f69167 commit 304ab8c

File tree

3 files changed

+106
-26
lines changed

3 files changed

+106
-26
lines changed

packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts

+25
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,29 @@ describe('stringify static html', () => {
168168
type: NodeTypes.VNODE_CALL
169169
})
170170
})
171+
172+
// #1128
173+
test('should bail on non attribute bindings', () => {
174+
const { ast } = compileWithStringify(
175+
`<div><div><input indeterminate>${repeat(
176+
`<span class="foo">foo</span>`,
177+
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
178+
)}</div></div>`
179+
)
180+
expect(ast.hoists.length).toBe(1)
181+
expect(ast.hoists[0]).toMatchObject({
182+
type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION
183+
})
184+
185+
const { ast: ast2 } = compileWithStringify(
186+
`<div><div><input :indeterminate="true">${repeat(
187+
`<span class="foo">foo</span>`,
188+
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
189+
)}</div></div>`
190+
)
191+
expect(ast2.hoists.length).toBe(1)
192+
expect(ast2.hoists[0]).toMatchObject({
193+
type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION
194+
})
195+
})
171196
})

packages/compiler-dom/src/transforms/stringifyStatic.ts

+40-15
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
/**
2+
* This module is Node-only.
3+
*/
14
import {
25
NodeTypes,
36
ElementNode,
@@ -13,6 +16,7 @@ import {
1316
isVoidTag,
1417
isString,
1518
isSymbol,
19+
isKnownAttr,
1620
escapeHtml,
1721
toDisplayString,
1822
normalizeClass,
@@ -38,6 +42,11 @@ export const enum StringifyThresholds {
3842
NODE_COUNT = 20
3943
}
4044

45+
const dataAriaRE = /^(data|aria)-/
46+
const isStringifiableAttr = (name: string) => {
47+
return isKnownAttr(name) || dataAriaRE.test(name)
48+
}
49+
4150
// Opt-in heuristics based on:
4251
// 1. number of elements with attributes > 5.
4352
// 2. OR: number of total nodes > 20
@@ -47,28 +56,44 @@ export const enum StringifyThresholds {
4756
function shouldOptimize(node: ElementNode): boolean {
4857
let bindingThreshold = StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
4958
let nodeThreshold = StringifyThresholds.NODE_COUNT
50-
let bail = false
59+
60+
let bailed = false
61+
const bail = () => {
62+
bailed = true
63+
return false
64+
}
5165

5266
// TODO: check for cases where using innerHTML will result in different
5367
// output compared to imperative node insertions.
5468
// probably only need to check for most common case
5569
// i.e. non-phrasing-content tags inside `<p>`
5670
function walk(node: ElementNode) {
57-
// some transforms, e.g. `transformAssetUrls` in `@vue/compiler-sfc` may
58-
// convert static attributes into a v-bind with a constnat expresion.
59-
// Such constant bindings are eligible for hoisting but not for static
60-
// stringification because they cannot be pre-evaluated.
6171
for (let i = 0; i < node.props.length; i++) {
6272
const p = node.props[i]
63-
if (
64-
p.type === NodeTypes.DIRECTIVE &&
65-
p.name === 'bind' &&
66-
p.exp &&
67-
p.exp.type !== NodeTypes.COMPOUND_EXPRESSION &&
68-
p.exp.isRuntimeConstant
69-
) {
70-
bail = true
71-
return false
73+
// bail on non-attr bindings
74+
if (p.type === NodeTypes.ATTRIBUTE && !isStringifiableAttr(p.name)) {
75+
return bail()
76+
}
77+
if (p.type === NodeTypes.DIRECTIVE && p.name === 'bind') {
78+
// bail on non-attr bindings
79+
if (
80+
p.arg &&
81+
(p.arg.type === NodeTypes.COMPOUND_EXPRESSION ||
82+
(p.arg.isStatic && !isStringifiableAttr(p.arg.content)))
83+
) {
84+
return bail()
85+
}
86+
// some transforms, e.g. `transformAssetUrls` in `@vue/compiler-sfc` may
87+
// convert static attributes into a v-bind with a constnat expresion.
88+
// Such constant bindings are eligible for hoisting but not for static
89+
// stringification because they cannot be pre-evaluated.
90+
if (
91+
p.exp &&
92+
(p.exp.type === NodeTypes.COMPOUND_EXPRESSION ||
93+
p.exp.isRuntimeConstant)
94+
) {
95+
return bail()
96+
}
7297
}
7398
}
7499
for (let i = 0; i < node.children.length; i++) {
@@ -83,7 +108,7 @@ function shouldOptimize(node: ElementNode): boolean {
83108
if (walk(child)) {
84109
return true
85110
}
86-
if (bail) {
111+
if (bailed) {
87112
return false
88113
}
89114
}

packages/shared/src/domAttrConfig.ts

+41-11
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
11
import { makeMap } from './makeMap'
22

3-
// On the client we only need to offer special cases for boolean attributes that
4-
// have different names from their corresponding dom properties:
5-
// - itemscope -> N/A
6-
// - allowfullscreen -> allowFullscreen
7-
// - formnovalidate -> formNoValidate
8-
// - ismap -> isMap
9-
// - nomodule -> noModule
10-
// - novalidate -> noValidate
11-
// - readonly -> readOnly
3+
/**
4+
* On the client we only need to offer special cases for boolean attributes that
5+
* have different names from their corresponding dom properties:
6+
* - itemscope -> N/A
7+
* - allowfullscreen -> allowFullscreen
8+
* - formnovalidate -> formNoValidate
9+
* - ismap -> isMap
10+
* - nomodule -> noModule
11+
* - novalidate -> noValidate
12+
* - readonly -> readOnly
13+
*/
1214
const specialBooleanAttrs = `itemscope,allowfullscreen,formnovalidate,ismap,nomodule,novalidate,readonly`
1315
export const isSpecialBooleanAttr = /*#__PURE__*/ makeMap(specialBooleanAttrs)
1416

15-
// The full list is needed during SSR to produce the correct initial markup.
17+
/**
18+
* The full list is needed during SSR to produce the correct initial markup.
19+
*/
1620
export const isBooleanAttr = /*#__PURE__*/ makeMap(
1721
specialBooleanAttrs +
1822
`,async,autofocus,autoplay,controls,default,defer,disabled,hidden,` +
@@ -41,7 +45,9 @@ export const propsToAttrMap: Record<string, string | undefined> = {
4145
httpEquiv: 'http-equiv'
4246
}
4347

44-
// CSS properties that accept plain numbers
48+
/**
49+
* CSS properties that accept plain numbers
50+
*/
4551
export const isNoUnitNumericStyleProp = /*#__PURE__*/ makeMap(
4652
`animation-iteration-count,border-image-outset,border-image-slice,` +
4753
`border-image-width,box-flex,box-flex-group,box-ordinal-group,column-count,` +
@@ -53,3 +59,27 @@ export const isNoUnitNumericStyleProp = /*#__PURE__*/ makeMap(
5359
`fill-opacity,flood-opacity,stop-opacity,stroke-dasharray,stroke-dashoffset,` +
5460
`stroke-miterlimit,stroke-opacity,stroke-width`
5561
)
62+
63+
/**
64+
* Known attributes, this is used for stringification of runtime static nodes
65+
* so that we don't stringify bindings that cannot be set from HTML.
66+
* Don't also forget to allow `data-*` and `aria-*`!
67+
* Generated from https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes
68+
*/
69+
export const isKnownAttr = /*#__PURE__*/ makeMap(
70+
`accept,accept-charset,accesskey,action,align,allow,alt,async,` +
71+
`autocapitalize,autocomplete,autofocus,autoplay,background,bgcolor,` +
72+
`border,buffered,capture,challenge,charset,checked,cite,class,code,` +
73+
`codebase,color,cols,colspan,content,contenteditable,contextmenu,controls,` +
74+
`coords,crossorigin,csp,data,datetime,decoding,default,defer,dir,dirname,` +
75+
`disabled,download,draggable,dropzone,enctype,enterkeyhint,for,form,` +
76+
`formaction,formenctype,formmethod,formnovalidate,formtarget,headers,` +
77+
`height,hidden,high,href,hreflang,http-equiv,icon,id,importance,integrity,` +
78+
`ismap,itemprop,keytype,kind,label,lang,language,loading,list,loop,low,` +
79+
`manifest,max,maxlength,minlength,media,min,multiple,muted,name,novalidate,` +
80+
`open,optimum,pattern,ping,placeholder,poster,preload,radiogroup,readonly,` +
81+
`referrerpolicy,rel,required,reversed,rows,rowspan,sandbox,scope,scoped,` +
82+
`selected,shape,size,sizes,slot,span,spellcheck,src,srcdoc,srclang,srcset,` +
83+
`start,step,style,summary,tabindex,target,title,translate,type,usemap,` +
84+
`value,width,wrap`
85+
)

0 commit comments

Comments
 (0)