From 3e8cd3f25fe8be230e6884e9fb4b9d5bceaf3035 Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 25 Sep 2019 12:39:46 -0400 Subject: [PATCH] wip(compiler): property deduping --- .../transforms/transformElement.spec.ts | 20 ++-- packages/compiler-core/src/ast.ts | 2 +- packages/compiler-core/src/codegen.ts | 2 +- .../src/transforms/transformElement.ts | 96 ++++++++++++++++++- .../src/transforms/transformExpression.ts | 9 +- .../src/transforms/vBindClass.ts | 9 -- .../src/transforms/vBindStyle.ts | 14 --- 7 files changed, 111 insertions(+), 41 deletions(-) delete mode 100644 packages/compiler-core/src/transforms/vBindClass.ts delete mode 100644 packages/compiler-core/src/transforms/vBindStyle.ts diff --git a/packages/compiler-core/__tests__/transforms/transformElement.spec.ts b/packages/compiler-core/__tests__/transforms/transformElement.spec.ts index 00152a751..137d373b8 100644 --- a/packages/compiler-core/__tests__/transforms/transformElement.spec.ts +++ b/packages/compiler-core/__tests__/transforms/transformElement.spec.ts @@ -3,7 +3,8 @@ import { CompilerOptions, parse, transform, - ErrorCodes + ErrorCodes, + compile } from '../../src' import { transformElement } from '../../src/transforms/transformElement' import { @@ -305,10 +306,7 @@ describe('compiler: element transform', () => { foo(dir) { _dir = dir return { - props: [ - createObjectProperty(dir.arg!, dir.exp!, dir.loc), - createObjectProperty(dir.arg!, dir.exp!, dir.loc) - ], + props: [createObjectProperty(dir.arg!, dir.exp!, dir.loc)], needRuntime: true } } @@ -328,11 +326,6 @@ describe('compiler: element transform', () => { { type: NodeTypes.JS_OBJECT_EXPRESSION, properties: [ - { - type: NodeTypes.JS_PROPERTY, - key: _dir!.arg, - value: _dir!.exp - }, { type: NodeTypes.JS_PROPERTY, key: _dir!.arg, @@ -457,5 +450,12 @@ describe('compiler: element transform', () => { ]) }) + test('props dedupe', () => { + const { code } = compile( + `
` + ) + console.log(code) + }) + test.todo('slot outlets') }) diff --git a/packages/compiler-core/src/ast.ts b/packages/compiler-core/src/ast.ts index 24b840110..de828ef6d 100644 --- a/packages/compiler-core/src/ast.ts +++ b/packages/compiler-core/src/ast.ts @@ -159,7 +159,7 @@ export interface ObjectExpression extends Node { export interface Property extends Node { type: NodeTypes.JS_PROPERTY key: ExpressionNode - value: ExpressionNode + value: JSChildNode } export interface ArrayExpression extends Node { diff --git a/packages/compiler-core/src/codegen.ts b/packages/compiler-core/src/codegen.ts index 62e831250..94d7241f6 100644 --- a/packages/compiler-core/src/codegen.ts +++ b/packages/compiler-core/src/codegen.ts @@ -453,7 +453,7 @@ function genObjectExpression(node: ObjectExpression, context: CodegenContext) { genExpressionAsPropertyKey(key, context) push(`: `) // value - genExpression(value, context) + genNode(value, context) if (i < properties.length - 1) { // will only reach this if it's multilines push(`,`) diff --git a/packages/compiler-core/src/transforms/transformElement.ts b/packages/compiler-core/src/transforms/transformElement.ts index 7b73411c7..fa58cb946 100644 --- a/packages/compiler-core/src/transforms/transformElement.ts +++ b/packages/compiler-core/src/transforms/transformElement.ts @@ -12,7 +12,8 @@ import { createArrayExpression, createObjectProperty, createExpression, - createObjectExpression + createObjectExpression, + Property } from '../ast' import { isArray } from '@vue/shared' import { createCompilerError, ErrorCodes } from '../errors' @@ -135,7 +136,9 @@ function buildProps( if (!arg && (isBind || name === 'on')) { if (exp) { if (properties.length) { - mergeArgs.push(createObjectExpression(properties, elementLoc)) + mergeArgs.push( + createObjectExpression(dedupeProperties(properties), elementLoc) + ) properties = [] } if (isBind) { @@ -187,7 +190,9 @@ function buildProps( // has v-bind="object" or v-on="object", wrap with mergeProps if (mergeArgs.length) { if (properties.length) { - mergeArgs.push(createObjectExpression(properties, elementLoc)) + mergeArgs.push( + createObjectExpression(dedupeProperties(properties), elementLoc) + ) } if (mergeArgs.length > 1) { context.imports.add(MERGE_PROPS) @@ -197,7 +202,10 @@ function buildProps( propsExpression = mergeArgs[0] } } else { - propsExpression = createObjectExpression(properties, elementLoc) + propsExpression = createObjectExpression( + dedupeProperties(properties), + elementLoc + ) } return { @@ -206,6 +214,86 @@ function buildProps( } } +// Dedupe props in an object literal. +// Literal duplicated attributes would have been warned during the parse phase, +// however, it's possible to encounter duplicated `onXXX` handlers with different +// modifiers. We also need to merge static and dynamic class / style attributes. +// - onXXX handlers / style: merge into array +// - class: merge into single expression with concatenation +function dedupeProperties(properties: Property[]): Property[] { + const knownProps: Record = {} + const deduped: Property[] = [] + for (let i = 0; i < properties.length; i++) { + const prop = properties[i] + // dynamic key named are always allowed + if (!prop.key.isStatic) { + deduped.push(prop) + continue + } + const name = prop.key.content + const existing = knownProps[name] + if (existing) { + if (name.startsWith('on')) { + mergeAsArray(existing, prop) + } else if (name === 'style') { + mergeStyles(existing, prop) + } else if (name === 'class') { + mergeClasses(existing, prop) + } + // unexpected duplicate, should have emitted error during parse + } else { + knownProps[name] = prop + deduped.push(prop) + } + } + return deduped +} + +function mergeAsArray(existing: Property, incoming: Property) { + if (existing.value.type === NodeTypes.JS_ARRAY_EXPRESSION) { + existing.value.elements.push(incoming.value) + } else { + existing.value = createArrayExpression( + [existing.value, incoming.value], + existing.loc + ) + } +} + +// Merge dynamic and static style into a single prop +export function mergeStyles(existing: Property, incoming: Property) { + if ( + existing.value.type === NodeTypes.JS_OBJECT_EXPRESSION && + incoming.value.type === NodeTypes.JS_OBJECT_EXPRESSION + ) { + // if both are objects, merge the object expressions. + // style="color: red" :style="{ a: b }" + // -> { color: "red", a: b } + existing.value.properties.push(...incoming.value.properties) + } else { + // otherwise merge as array + // style="color:red" :style="a" + // -> style: [{ color: "red" }, a] + mergeAsArray(existing, incoming) + } +} + +// Merge dynamic and static class into a single prop +function mergeClasses(existing: Property, incoming: Property) { + const e = existing.value as ExpressionNode + const children = + e.children || + (e.children = [ + { + ...e, + children: undefined + } + ]) + // :class="expression" class="string" + // -> class: expression + "string" + children.push(` + " " + `, incoming.value as ExpressionNode) +} + function createDirectiveArgs( dir: DirectiveNode, context: TransformContext diff --git a/packages/compiler-core/src/transforms/transformExpression.ts b/packages/compiler-core/src/transforms/transformExpression.ts index 12dbf5151..224438fa6 100644 --- a/packages/compiler-core/src/transforms/transformExpression.ts +++ b/packages/compiler-core/src/transforms/transformExpression.ts @@ -14,7 +14,6 @@ import { NodeTransform, TransformContext } from '../transform' import { NodeTypes, createExpression, ExpressionNode } from '../ast' import { Node, Function, Identifier } from 'estree' import { advancePositionWithClone } from '../utils' - export const transformExpression: NodeTransform = (node, context) => { if (node.type === NodeTypes.EXPRESSION && !node.isStatic) { processExpression(node, context) @@ -27,8 +26,14 @@ export const transformExpression: NodeTransform = (node, context) => { processExpression(prop.exp, context) } if (prop.arg && !prop.arg.isStatic) { - processExpression(prop.arg, context) + if (prop.name === 'class') { + // TODO special expression optimization for classes + } else { + processExpression(prop.arg, context) + } } + } else if (prop.name === 'style') { + // TODO parse inline CSS literals into objects } } } diff --git a/packages/compiler-core/src/transforms/vBindClass.ts b/packages/compiler-core/src/transforms/vBindClass.ts deleted file mode 100644 index d5392cfb1..000000000 --- a/packages/compiler-core/src/transforms/vBindClass.ts +++ /dev/null @@ -1,9 +0,0 @@ -// Optimizations -// - b -> b (use runtime normalization) -// - ['foo', b] -> 'foo' + normalize(b) -// - { a, b: c } -> (a ? a : '') + (b ? c : '') -// - ['a', b, { c }] -> 'a' + normalize(b) + (c ? c : '') - -// Also merge dynamic and static class into a single prop - -// Attach CLASS patchFlag if necessary diff --git a/packages/compiler-core/src/transforms/vBindStyle.ts b/packages/compiler-core/src/transforms/vBindStyle.ts deleted file mode 100644 index b3f23cf48..000000000 --- a/packages/compiler-core/src/transforms/vBindStyle.ts +++ /dev/null @@ -1,14 +0,0 @@ -// Optimizations -// The compiler pre-compiles static string styles into static objects -// + detects and hoists inline static objects - -// e.g. `style="color: red"` and `:style="{ color: 'red' }"` both get hoisted as - -// ``` js -// const style = { color: 'red' } -// render() { return e('div', { style }) } -// ``` - -// Also nerge dynamic and static style into a single prop - -// Attach STYLE patchFlag if necessary