refactor: setDynamicProp(s) need return prev value

vuejs · Dec 6, 2024 · 24fa426 · 24fa426
1 parent e979308
commit 24fa426
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 61 deletions.
diff --git a/packages/compiler-vapor/__tests__/__snapshots__/compile.spec.ts.snap b/packages/compiler-vapor/__tests__/__snapshots__/compile.spec.ts.snap
@@ -210,8 +210,8 @@ exports[`compile > expression parsing > v-bind 1`] = `
 "((_ctx) => {
   const n0 = t0()
   _setInheritAttrs(true)
-  let _key_value, _foo
-  _renderEffect(() => (_key_value !== key.value || _foo !== _unref(foo)) && _setDynamicProps(n0, [{ [(_key_value = key.value)+1]: (_foo = _unref(foo))[key.value+1]() }], true))
+  let _key_value, _foo, _key_value_foo
+  _renderEffect(() => (_key_value !== key.value || _foo !== _unref(foo)) && (_key_value_foo = _setDynamicProps(n0, _key_value_foo, [{ [key.value+1]: _unref(foo)[key.value+1]() }], true)))
   return n0
 })()"
 `;

diff --git a/packages/compiler-vapor/__tests__/compile.spec.ts b/packages/compiler-vapor/__tests__/compile.spec.ts
@@ -195,7 +195,7 @@ describe('compile', () => {
       expect(code).matchSnapshot()
       expect(code).contains('key.value+1')
       expect(code).contains(
-        '(_key_value !== key.value || _foo !== _unref(foo)) && _setDynamicProps(n0, [{ [(_key_value = key.value)+1]: (_foo = _unref(foo))[key.value+1]() }], true)',
+        '(_key_value !== key.value || _foo !== _unref(foo)) && (_key_value_foo = _setDynamicProps(n0, _key_value_foo, [{ [key.value+1]: _unref(foo)[key.value+1]() }], true))',
       )
     })
 

diff --git a/packages/compiler-vapor/__tests__/transforms/__snapshots__/transformElement.spec.ts.snap b/packages/compiler-vapor/__tests__/transforms/__snapshots__/transformElement.spec.ts.snap
@@ -357,7 +357,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
   let _obj
-  _renderEffect(() => _obj !== _ctx.obj && _setDynamicProps(n0, [(_obj = _ctx.obj)], true))
+  _renderEffect(() => _obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [_ctx.obj], true)))
   return n0
 }"
 `;
@@ -370,7 +370,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
   let _obj
-  _renderEffect(() => _obj !== _ctx.obj && _setDynamicProps(n0, [{ id: "foo" }, (_obj = _ctx.obj)], true))
+  _renderEffect(() => _obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [{ id: "foo" }, _ctx.obj], true)))
   return n0
 }"
 `;
@@ -383,7 +383,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
   let _obj
-  _renderEffect(() => _obj !== _ctx.obj && _setDynamicProps(n0, [(_obj = _ctx.obj), { id: "foo" }], true))
+  _renderEffect(() => _obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [_ctx.obj, { id: "foo" }], true)))
   return n0
 }"
 `;
@@ -396,7 +396,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
   let _obj
-  _renderEffect(() => _obj !== _ctx.obj && _setDynamicProps(n0, [{ id: "foo" }, (_obj = _ctx.obj), { class: "bar" }], true))
+  _renderEffect(() => _obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [{ id: "foo" }, _ctx.obj, { class: "bar" }], true)))
   return n0
 }"
 `;

diff --git a/packages/compiler-vapor/__tests__/transforms/__snapshots__/vBind.spec.ts.snap b/packages/compiler-vapor/__tests__/transforms/__snapshots__/vBind.spec.ts.snap
@@ -86,7 +86,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(["fooBar"])
   let _id
-  _renderEffect(() => _id !== _ctx.id && _setDynamicProp(n0, "fooBar", _id, (_id = _ctx.id)))
+  _renderEffect(() => _id !== _ctx.id && (_id = _setDynamicProp(n0, "fooBar", _id, _ctx.id)))
   return n0
 }"
 `;
@@ -99,8 +99,8 @@ const t0 = _template("<div></div>")
 export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
-  let _foo, _id
-  _renderEffect(() => (_foo !== _ctx.foo || _id !== _ctx.id) && _setDynamicProps(n0, [{ [_camelize((_foo = _ctx.foo))]: (_id = _ctx.id) }], true))
+  let _foo, _id, _foo_id
+  _renderEffect(() => (_foo !== _ctx.foo || _id !== _ctx.id) && (_foo_id = _setDynamicProps(n0, _foo_id, [{ [_camelize(_ctx.foo)]: _ctx.id }], true)))
   return n0
 }"
 `;
@@ -113,7 +113,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(["fooBar"])
   let _fooBar
-  _renderEffect(() => _fooBar !== _ctx.fooBar && _setDynamicProp(n0, "fooBar", _fooBar, (_fooBar = _ctx.fooBar)))
+  _renderEffect(() => _fooBar !== _ctx.fooBar && (_fooBar = _setDynamicProp(n0, "fooBar", _fooBar, _ctx.fooBar)))
   return n0
 }"
 `;
@@ -216,8 +216,8 @@ const t0 = _template("<div></div>")
 export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
-  let _fooBar, _id
-  _renderEffect(() => (_fooBar !== _ctx.fooBar || _id !== _ctx.id) && _setDynamicProps(n0, [{ ["." + (_fooBar = _ctx.fooBar)]: (_id = _ctx.id) }], true))
+  let _fooBar, _id, _fooBar_id
+  _renderEffect(() => (_fooBar !== _ctx.fooBar || _id !== _ctx.id) && (_fooBar_id = _setDynamicProps(n0, _fooBar_id, [{ ["." + _ctx.fooBar]: _ctx.id }], true)))
   return n0
 }"
 `;
@@ -334,7 +334,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(["value"])
   let _foo
-  _renderEffect(() => _foo !== _ctx.foo && _setDynamicProp(n0, "value", _foo, (_foo = _ctx.foo)))
+  _renderEffect(() => _foo !== _ctx.foo && (_foo = _setDynamicProp(n0, "value", _foo, _ctx.foo)))
   return n0
 }"
 `;
@@ -459,8 +459,8 @@ const t0 = _template("<div></div>")
 export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
-  let _id, _title
-  _renderEffect(() => (_id !== _ctx.id || _title !== _ctx.title) && _setDynamicProps(n0, [{ [(_id = _ctx.id)]: _ctx.id, [(_title = _ctx.title)]: _ctx.title }], true))
+  let _id, _title, _id_title
+  _renderEffect(() => (_id !== _ctx.id || _title !== _ctx.title) && (_id_title = _setDynamicProps(n0, _id_title, [{ [_ctx.id]: _ctx.id, [_ctx.title]: _ctx.title }], true)))
   return n0
 }"
 `;
@@ -473,7 +473,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(true)
   let _id
-  _renderEffect(() => _id !== _ctx.id && _setDynamicProps(n0, [{ [(_id = _ctx.id)]: _ctx.id, foo: "bar", checked: "" }], true))
+  _renderEffect(() => _id !== _ctx.id && (_id = _setDynamicProps(n0, _id, [{ [_ctx.id]: _ctx.id, foo: "bar", checked: "" }], true)))
   return n0
 }"
 `;
@@ -486,7 +486,7 @@ export function render(_ctx) {
   const n0 = t0()
   _setInheritAttrs(["camel-case"])
   let _camelCase
-  _renderEffect(() => _camelCase !== _ctx.camelCase && _setDynamicProp(n0, "camel-case", _camelCase, (_camelCase = _ctx.camelCase)))
+  _renderEffect(() => _camelCase !== _ctx.camelCase && (_camelCase = _setDynamicProp(n0, "camel-case", _camelCase, _ctx.camelCase)))
   return n0
 }"
 `;

diff --git a/packages/compiler-vapor/__tests__/transforms/__snapshots__/vModel.spec.ts.snap b/packages/compiler-vapor/__tests__/transforms/__snapshots__/vModel.spec.ts.snap
@@ -255,7 +255,7 @@ export function render(_ctx) {
   _delegate(n0, "update:modelValue", () => $event => (_ctx.model = $event))
   _setInheritAttrs(true)
   let _obj
-  _renderEffect(() => _obj !== _ctx.obj && _setDynamicProps(n0, [(_obj = _ctx.obj)], true))
+  _renderEffect(() => _obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [_ctx.obj], true)))
   return n0
 }"
 `;

diff --git a/packages/compiler-vapor/__tests__/transforms/transformElement.spec.ts b/packages/compiler-vapor/__tests__/transforms/transformElement.spec.ts
@@ -589,7 +589,7 @@ describe('compiler: element transform', () => {
       },
     ])
     expect(code).contains(
-      '_obj !== _ctx.obj && _setDynamicProps(n0, [(_obj = _ctx.obj)], true)',
+      '_obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [_ctx.obj], true))',
     )
   })
 
@@ -627,7 +627,7 @@ describe('compiler: element transform', () => {
       },
     ])
     expect(code).contains(
-      '_obj !== _ctx.obj && _setDynamicProps(n0, [{ id: "foo" }, (_obj = _ctx.obj)], true)',
+      '_obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [{ id: "foo" }, _ctx.obj], true))',
     )
   })
 
@@ -655,7 +655,7 @@ describe('compiler: element transform', () => {
       },
     ])
     expect(code).contains(
-      '_obj !== _ctx.obj && _setDynamicProps(n0, [(_obj = _ctx.obj), { id: "foo" }], true)',
+      '_obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [_ctx.obj, { id: "foo" }], true))',
     )
   })
 
@@ -684,7 +684,7 @@ describe('compiler: element transform', () => {
       },
     ])
     expect(code).contains(
-      '_obj !== _ctx.obj && _setDynamicProps(n0, [{ id: "foo" }, (_obj = _ctx.obj), { class: "bar" }], true)',
+      '_obj !== _ctx.obj && (_obj = _setDynamicProps(n0, _obj, [{ id: "foo" }, _ctx.obj, { class: "bar" }], true))',
     )
   })
 

diff --git a/packages/compiler-vapor/__tests__/transforms/vBind.spec.ts b/packages/compiler-vapor/__tests__/transforms/vBind.spec.ts
@@ -131,7 +131,7 @@ describe('compiler v-bind', () => {
       },
     })
     expect(code).contains(
-      '_camelCase !== _ctx.camelCase && _setDynamicProp(n0, "camel-case", _camelCase, (_camelCase = _ctx.camelCase))',
+      '_camelCase !== _ctx.camelCase && (_camelCase = _setDynamicProp(n0, "camel-case", _camelCase, _ctx.camelCase))',
     )
   })
 
@@ -177,7 +177,7 @@ describe('compiler v-bind', () => {
       ],
     })
     expect(code).contains(
-      '(_id !== _ctx.id || _title !== _ctx.title) && _setDynamicProps(n0, [{ [(_id = _ctx.id)]: _ctx.id, [(_title = _ctx.title)]: _ctx.title }], true)',
+      '(_id !== _ctx.id || _title !== _ctx.title) && (_id_title = _setDynamicProps(n0, _id_title, [{ [_ctx.id]: _ctx.id, [_ctx.title]: _ctx.title }], true))',
     )
   })
 
@@ -230,7 +230,7 @@ describe('compiler v-bind', () => {
       ],
     })
     expect(code).contains(
-      '_id !== _ctx.id && _setDynamicProps(n0, [{ [(_id = _ctx.id)]: _ctx.id, foo: "bar", checked: "" }], true)',
+      '_id !== _ctx.id && (_id = _setDynamicProps(n0, _id, [{ [_ctx.id]: _ctx.id, foo: "bar", checked: "" }], true))',
     )
   })
 
@@ -293,7 +293,7 @@ describe('compiler v-bind', () => {
 
     expect(code).matchSnapshot()
     expect(code).contains(
-      '_id !== _ctx.id && _setDynamicProp(n0, "fooBar", _id, (_id = _ctx.id))',
+      '_id !== _ctx.id && (_id = _setDynamicProp(n0, "fooBar", _id, _ctx.id))',
     )
   })
 
@@ -319,7 +319,7 @@ describe('compiler v-bind', () => {
     })
     expect(code).contains('renderEffect')
     expect(code).contains(
-      '_fooBar !== _ctx.fooBar && _setDynamicProp(n0, "fooBar", _fooBar, (_fooBar = _ctx.fooBar))',
+      '_fooBar !== _ctx.fooBar && (_fooBar = _setDynamicProp(n0, "fooBar", _fooBar, _ctx.fooBar))',
     )
   })
 
@@ -351,7 +351,7 @@ describe('compiler v-bind', () => {
     expect(code).matchSnapshot()
     expect(code).contains('renderEffect')
     expect(code).contains(
-      `(_foo !== _ctx.foo || _id !== _ctx.id) && _setDynamicProps(n0, [{ [_camelize((_foo = _ctx.foo))]: (_id = _ctx.id) }], true)`,
+      `(_foo !== _ctx.foo || _id !== _ctx.id) && (_foo_id = _setDynamicProps(n0, _foo_id, [{ [_camelize(_ctx.foo)]: _ctx.id }], true))`,
     )
   })
 
@@ -436,7 +436,7 @@ describe('compiler v-bind', () => {
     })
     expect(code).contains('renderEffect')
     expect(code).contains(
-      `(_fooBar !== _ctx.fooBar || _id !== _ctx.id) && _setDynamicProps(n0, [{ ["." + (_fooBar = _ctx.fooBar)]: (_id = _ctx.id) }], true)`,
+      `(_fooBar !== _ctx.fooBar || _id !== _ctx.id) && (_fooBar_id = _setDynamicProps(n0, _fooBar_id, [{ ["." + _ctx.fooBar]: _ctx.id }], true))`,
     )
   })
 
@@ -788,7 +788,7 @@ describe('compiler v-bind', () => {
     `)
     expect(code).matchSnapshot()
     expect(code).contains(
-      '_foo !== _ctx.foo && _setDynamicProp(n0, "value", _foo, (_foo = _ctx.foo))',
+      '_foo !== _ctx.foo && (_foo = _setDynamicProp(n0, "value", _foo, _ctx.foo))',
     )
   })
 

diff --git a/packages/compiler-vapor/src/generators/prop.ts b/packages/compiler-vapor/src/generators/prop.ts
@@ -35,7 +35,8 @@ import {
   toHandlerKey,
 } from '@vue/shared'
 
-const helperNeedPrevValue = ['setStyle', 'setDynamicProp']
+// those runtime helpers will return the prevValue
+const helperNeedPrevValue = ['setStyle', 'setDynamicProp', 'setDynamicProps']
 
 // only the static key prop will reach here
 export function genSetProp(
@@ -51,37 +52,32 @@ export function genSetProp(
   const { helperName, omitKey } = getRuntimeHelper(tag, key.content, modifier)
   let propValue = genPropValue(values, context)
 
+  let prevValueName
   if (shouldCacheRenderEffectDeps()) {
-    processValues(context, [propValue])
-
+    const needReturnValue = helperNeedPrevValue.includes(helperName)
+    processValues(context, [propValue], !needReturnValue)
     const { declareNames } = processingRenderEffect!
-    // need prevValue parameter
-    if (declareNames.size > 0 && helperNeedPrevValue.includes(helperName)) {
-      const prevValueName = [...declareNames].join('')
+    if (declareNames.size > 0 && needReturnValue) {
+      prevValueName = [...declareNames].join('')
       declareNames.add(prevValueName)
-      const needReCacheValue = declareNames.size > 1
-      propValue.unshift(
-        ...[
-          `${prevValueName}, `, // prevValue parameter
-          needReCacheValue ? `(${prevValueName} = ` : undefined, // cache value to prevValue
-        ],
-      )
-      needReCacheValue && propValue.push(')')
     }
   }
 
   return [
     NEWLINE,
+    ...(prevValueName ? [`(`, `${prevValueName} = `] : []),
     ...genCall(
       [vaporHelper(helperName), null],
       `n${oper.element}`,
       omitKey ? false : genExpression(key, context),
+      ...(prevValueName ? [`${prevValueName}`] : []),
       propValue,
       // only `setClass` and `setStyle` need merge inherit attr
       oper.root && (helperName === 'setClass' || helperName === 'setStyle')
         ? 'true'
         : undefined,
     ),
+    ...(prevValueName ? [`)`] : []),
   ]
 }
 
@@ -90,7 +86,8 @@ export function genDynamicProps(
   oper: SetDynamicPropsIRNode,
   context: CodegenContext,
 ): CodeFragment[] {
-  const { vaporHelper, shouldCacheRenderEffectDeps } = context
+  const { vaporHelper, shouldCacheRenderEffectDeps, processingRenderEffect } =
+    context
   const values = oper.props.map(props =>
     Array.isArray(props)
       ? genLiteralObjectProps(props, context) // static and dynamic arg props
@@ -99,18 +96,27 @@ export function genDynamicProps(
         : genExpression(props.value, context),
   ) // v-bind=""
 
+  let prevValueName
   if (shouldCacheRenderEffectDeps()) {
-    processValues(context, values)
+    processValues(context, values, false)
+    const { declareNames } = processingRenderEffect!
+    if (declareNames.size > 0) {
+      prevValueName = [...declareNames].join('')
+      declareNames.add(prevValueName)
+    }
   }
 
   return [
     NEWLINE,
+    ...(prevValueName ? [`(`, `${prevValueName} = `] : []),
     ...genCall(
       vaporHelper('setDynamicProps'),
       `n${oper.element}`,
+      ...(prevValueName ? [`${prevValueName}`] : []),
       genMulti(DELIMITERS_ARRAY, ...values),
       oper.root && 'true',
     ),
+    ...(prevValueName ? [`)`] : []),
   ]
 }
 
@@ -265,10 +271,11 @@ const getSpecialHelper = (
 export function processValues(
   context: CodegenContext,
   values: CodeFragment[][],
+  needRewrite: boolean = true,
 ): string[] {
   const allCheckExps: string[] = []
   values.forEach(value => {
-    const checkExps = processValue(context, value)
+    const checkExps = processValue(context, value, needRewrite)
     if (checkExps) allCheckExps.push(...checkExps, ' && ')
   })
 
@@ -282,12 +289,13 @@ export function processValues(
 function processValue(
   context: CodegenContext,
   values: CodeFragment[],
+  needRewrite: boolean = true,
 ): string[] | undefined {
   const { processingRenderEffect, allRenderEffectSeenNames } = context
   const { declareNames, rewrittenNames, earlyCheckExps, operations } =
     processingRenderEffect!
 
-  const isMultiLine = operations.length > 1
+  const isSingleLine = operations.length === 1
   for (const frag of values) {
     if (!isArray(frag)) continue
     // [code, newlineIndex, loc, name] -> [(_name = code), newlineIndex, loc, name]
@@ -307,7 +315,7 @@ function processValue(
       declareNames.add(name)
       earlyCheckExps.push(`${name} !== ${newName}`)
 
-      if (!isMultiLine) {
+      if (needRewrite && isSingleLine) {
         // replace the original code fragment with the assignment expression
         frag[0] = `(${name} = ${newName})`
       }