From 4c6078ce25226ab9e10ec4eba5c745058f670b3d Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 18 Nov 2020 22:39:08 -0500 Subject: [PATCH] fix(compiler-core/compiler-sfc): handle destructure assignment expressions --- .../src/transforms/transformExpression.ts | 63 ++++++++++++++----- .../__snapshots__/compileScript.spec.ts.snap | 58 +++++++++++++---- .../__tests__/compileScript.spec.ts | 61 ++++++++++++++---- packages/compiler-sfc/src/compileScript.ts | 47 +++++++++++--- packages/template-explorer/src/options.ts | 8 ++- 5 files changed, 188 insertions(+), 49 deletions(-) diff --git a/packages/compiler-core/src/transforms/transformExpression.ts b/packages/compiler-core/src/transforms/transformExpression.ts index 4ad7d8884a6..83f80e87553 100644 --- a/packages/compiler-core/src/transforms/transformExpression.ts +++ b/packages/compiler-core/src/transforms/transformExpression.ts @@ -111,22 +111,26 @@ export function processExpression( const rewriteIdentifier = (raw: string, parent?: Node, id?: Identifier) => { const type = hasOwn(bindingMetadata, raw) && bindingMetadata[raw] if (inline) { + // x = y const isAssignmentLVal = parent && parent.type === 'AssignmentExpression' && parent.left === id + // x++ const isUpdateArg = parent && parent.type === 'UpdateExpression' && parent.argument === id - // setup inline mode + // ({ x } = y) + const isDestructureAssignment = + parent && isInDestructureAssignment(parent, parentStack) + if (type === BindingTypes.SETUP_CONST) { return raw - } else if ( - type === BindingTypes.SETUP_REF || - type === BindingTypes.SETUP_MAYBE_REF - ) { + } else if (type === BindingTypes.SETUP_REF) { + return `${raw}.value` + } else if (type === BindingTypes.SETUP_MAYBE_REF) { // const binding that may or may not be ref // if it's not a ref, then assignments don't make sense - // so we ignore the non-ref assignment case and generate code // that assumes the value to be a ref for more efficiency - return isAssignmentLVal || isUpdateArg + return isAssignmentLVal || isUpdateArg || isDestructureAssignment ? `${raw}.value` : `${context.helperString(UNREF)}(${raw})` } else if (type === BindingTypes.SETUP_LET) { @@ -157,6 +161,13 @@ export function processExpression( return `${context.helperString(IS_REF)}(${raw})${ context.isTS ? ` //@ts-ignore\n` : `` } ? ${prefix}${raw}.value${postfix} : ${prefix}${raw}${postfix}` + } else if (isDestructureAssignment) { + // TODO + // let binding in a destructure assignment - it's very tricky to + // handle both possible cases here without altering the original + // structure of the code, so we just assume it's not a ref here + // for now + return raw } else { return `${context.helperString(UNREF)}(${raw})` } @@ -231,22 +242,24 @@ export function processExpression( const knownIds = Object.create(context.identifiers) const isDuplicate = (node: Node & PrefixMeta): boolean => ids.some(id => id.start === node.start) + const parentStack: Node[] = [] // walk the AST and look for identifiers that need to be prefixed. ;(walk as any)(ast, { - enter(node: Node & PrefixMeta, parent: Node) { + enter(node: Node & PrefixMeta, parent: Node | undefined) { + parent && parentStack.push(parent) if (node.type === 'Identifier') { if (!isDuplicate(node)) { - const needPrefix = shouldPrefix(node, parent) + const needPrefix = shouldPrefix(node, parent!, parentStack) if (!knownIds[node.name] && needPrefix) { - if (isStaticProperty(parent) && parent.shorthand) { + if (isStaticProperty(parent!) && parent.shorthand) { // property shorthand like { foo }, we need to add the key since // we rewrite the value node.prefix = `${node.name}: ` } node.name = rewriteIdentifier(node.name, parent, node) ids.push(node) - } else if (!isStaticPropertyKey(node, parent)) { + } else if (!isStaticPropertyKey(node, parent!)) { // The identifier is considered constant unless it's pointing to a // scope variable (a v-for alias, or a v-slot prop) if (!(needPrefix && knownIds[node.name]) && !bailConstant) { @@ -291,7 +304,8 @@ export function processExpression( ) } }, - leave(node: Node & PrefixMeta) { + leave(node: Node & PrefixMeta, parent: Node | undefined) { + parent && parentStack.pop() if (node !== ast.body[0].expression && node.scopeIds) { node.scopeIds.forEach((id: string) => { knownIds[id]-- @@ -359,7 +373,7 @@ const isStaticProperty = (node: Node): node is ObjectProperty => const isStaticPropertyKey = (node: Node, parent: Node) => isStaticProperty(parent) && parent.key === node -function shouldPrefix(id: Identifier, parent: Node) { +function shouldPrefix(id: Identifier, parent: Node, parentStack: Node[]) { // declaration id if ( (parent.type === 'VariableDeclarator' || @@ -386,8 +400,11 @@ function shouldPrefix(id: Identifier, parent: Node) { return false } - // array destructure pattern - if (parent.type === 'ArrayPattern') { + // non-assignment array destructure pattern + if ( + parent.type === 'ArrayPattern' && + !isInDestructureAssignment(parent, parentStack) + ) { return false } @@ -419,6 +436,24 @@ function shouldPrefix(id: Identifier, parent: Node) { return true } +function isInDestructureAssignment(parent: Node, parentStack: Node[]): boolean { + if ( + parent && + (parent.type === 'ObjectProperty' || parent.type === 'ArrayPattern') + ) { + let i = parentStack.length + while (i--) { + const p = parentStack[i] + if (p.type === 'AssignmentExpression') { + return true + } else if (p.type !== 'ObjectProperty' && !p.type.endsWith('Pattern')) { + break + } + } + } + return false +} + function stringifyExpression(exp: ExpressionNode | string): string { if (isString(exp)) { return exp diff --git a/packages/compiler-sfc/__tests__/__snapshots__/compileScript.spec.ts.snap b/packages/compiler-sfc/__tests__/__snapshots__/compileScript.spec.ts.snap index a08802a8c58..6ff7608ccfe 100644 --- a/packages/compiler-sfc/__tests__/__snapshots__/compileScript.spec.ts.snap +++ b/packages/compiler-sfc/__tests__/__snapshots__/compileScript.spec.ts.snap @@ -117,7 +117,7 @@ return { ref } `; exports[`SFC compile `, { inlineTemplate: true } ) - assertCode(content) // no need to unref vue component import expect(content).toMatch(`createVNode(Foo)`) // should unref other imports expect(content).toMatch(`unref(other)`) // no need to unref constant literals expect(content).not.toMatch(`unref(constant)`) - // should unref const w/ call init (e.g. ref()) - expect(content).toMatch(`unref(count)`) + // should directly use .value for known refs + expect(content).toMatch(`count.value`) + // should unref() on const bindings that may be refs + expect(content).toMatch(`unref(maybe)`) + // should unref() on let bindings + expect(content).toMatch(`unref(lett)`) // no need to unref function declarations expect(content).toMatch(`{ onClick: fn }`) // no need to mark constant fns in patch flag expect(content).not.toMatch(`PROPS`) + assertCode(content) }) test('v-model codegen', () => { @@ -170,8 +176,9 @@ const bar = 1 ) // known const ref: set value expect(content).toMatch(`count.value = $event`) - // const but maybe ref: only assign after check - expect(content).toMatch(`_isRef(maybe) ? maybe.value = $event : null`) + // const but maybe ref: also assign .value directly since non-ref + // won't work + expect(content).toMatch(`maybe.value = $event`) // let: handle both cases expect(content).toMatch( `_isRef(lett) ? lett.value = $event : lett = $event` @@ -198,12 +205,10 @@ const bar = 1 // known const ref: set value expect(content).toMatch(`count.value = 1`) // const but maybe ref: only assign after check - expect(content).toMatch( - `!_isRef(maybe) ? null : maybe.value = _unref(count)` - ) + expect(content).toMatch(`maybe.value = count.value`) // let: handle both cases expect(content).toMatch( - `_isRef(lett) ? lett.value = _unref(count) : lett = _unref(count)` + `_isRef(lett) ? lett.value = count.value : lett = count.value` ) assertCode(content) }) @@ -230,14 +235,40 @@ const bar = 1 // known const ref: set value expect(content).toMatch(`count.value++`) expect(content).toMatch(`--count.value`) - // const but maybe ref: only assign after check - expect(content).toMatch(`!_isRef(maybe) ? null : maybe.value++`) - expect(content).toMatch(`!_isRef(maybe) ? null : --maybe.value`) + // const but maybe ref (non-ref case ignored) + expect(content).toMatch(`maybe.value++`) + expect(content).toMatch(`--maybe.value`) // let: handle both cases expect(content).toMatch(`_isRef(lett) ? lett.value++ : lett++`) expect(content).toMatch(`_isRef(lett) ? --lett.value : --lett`) assertCode(content) }) + + test('template destructure assignment codegen', () => { + const { content } = compile( + ` + + `, + { inlineTemplate: true } + ) + // known const ref: set value + expect(content).toMatch(`({ count: count.value } = val)`) + // const but maybe ref (non-ref case ignored) + expect(content).toMatch(`[maybe.value] = val`) + // let: assumes non-ref + expect(content).toMatch(`{ lett: lett } = val`) + assertCode(content) + }) }) describe('with TypeScript', () => { @@ -524,12 +555,16 @@ const { props, emit } = defineOptions({ a = a + 1 b.count++ b.count = b.count + 1 + ;({ a } = { a: 2 }) + ;[a] = [1] } `) expect(content).toMatch(`a.value++`) expect(content).toMatch(`a.value = a.value + 1`) expect(content).toMatch(`b.value.count++`) expect(content).toMatch(`b.value.count = b.value.count + 1`) + expect(content).toMatch(`;({ a: a.value } = { a: 2 })`) + expect(content).toMatch(`;[a.value] = [1]`) assertCode(content) }) diff --git a/packages/compiler-sfc/src/compileScript.ts b/packages/compiler-sfc/src/compileScript.ts index 4e1d1288df2..78579be5efc 100644 --- a/packages/compiler-sfc/src/compileScript.ts +++ b/packages/compiler-sfc/src/compileScript.ts @@ -670,7 +670,10 @@ export function compileScript( // let binding used in a property shorthand // { foo } -> { foo: foo.value } // skip for destructure patterns - if (!(parent as any).inPattern) { + if ( + !(parent as any).inPattern || + isInDestructureAssignment(parent, parentStack) + ) { s.appendLeft(id.end! + startOffset, `: ${id.name}.value`) } } else { @@ -1258,6 +1261,8 @@ function genRuntimeEmits(emits: Set) { : `` } +const parentStack: Node[] = [] + /** * Walk an AST and find identifiers that are variable references. * This is largely the same logic with `transformExpressions` in compiler-core @@ -1270,10 +1275,11 @@ function walkIdentifiers( ) { const knownIds: Record = Object.create(null) ;(walk as any)(root, { - enter(node: Node & { scopeIds?: Set }, parent: Node) { + enter(node: Node & { scopeIds?: Set }, parent: Node | undefined) { + parent && parentStack.push(parent) if (node.type === 'Identifier') { - if (!knownIds[node.name] && isRefIdentifier(node, parent)) { - onIdentifier(node, parent) + if (!knownIds[node.name] && isRefIdentifier(node, parent!)) { + onIdentifier(node, parent!) } } else if (isFunction(node)) { // walk function expressions and add its arguments to known identifiers @@ -1309,13 +1315,14 @@ function walkIdentifiers( ) } else if ( node.type === 'ObjectProperty' && - parent.type === 'ObjectPattern' + parent!.type === 'ObjectPattern' ) { // mark property in destructure pattern ;(node as any).inPattern = true } }, - leave(node: Node & { scopeIds?: Set }) { + leave(node: Node & { scopeIds?: Set }, parent: Node | undefined) { + parent && parentStack.pop() if (node.scopeIds) { node.scopeIds.forEach((id: string) => { knownIds[id]-- @@ -1355,8 +1362,11 @@ function isRefIdentifier(id: Identifier, parent: Node) { return false } - // array destructure pattern - if (parent.type === 'ArrayPattern') { + // non-assignment array destructure pattern + if ( + parent.type === 'ArrayPattern' && + !isInDestructureAssignment(parent, parentStack) + ) { return false } @@ -1464,6 +1474,27 @@ function canNeverBeRef(node: Node, userReactiveImport: string): boolean { } } +function isInDestructureAssignment(parent: Node, parentStack: Node[]): boolean { + if ( + parent && + (parent.type === 'ObjectProperty' || parent.type === 'ArrayPattern') + ) { + let i = parentStack.length + while (i--) { + const p = parentStack[i] + if (p.type === 'AssignmentExpression') { + const root = parentStack[0] + // if this is a ref: destructure, it should be treated like a + // variable decalration! + return !(root.type === 'LabeledStatement' && root.label.name === 'ref') + } else if (p.type !== 'ObjectProperty' && !p.type.endsWith('Pattern')) { + break + } + } + } + return false +} + /** * Analyze bindings in normal `