Server Actions: Fix member expr in closure captured values (#50020)

For static member expressions like `foo.bar`, we currently treat it as one whole Ident `foo.bar` and pass it over the `$$bound` array as one value. That causes a problem where the Ident inside the function body needs to be converted, because it can no longer access to `foo`. Closes #49985. fix NEXT-1206
vercel · May 19, 2023 · 771141d · 771141d
1 parent 1dcff1d
commit 771141d
Show file tree

Hide file tree

Showing 12 changed files with 119 additions and 45 deletions.
diff --git a/packages/next-swc/crates/core/src/server_actions.rs b/packages/next-swc/crates/core/src/server_actions.rs
@@ -220,6 +220,12 @@ impl<C: Comments> ServerActions<C> {
                 Some(action_ident.clone()),
             );
 
+            if let BlockStmtOrExpr::BlockStmt(block) = &mut *a.body {
+                block.visit_mut_with(&mut ClosureReplacer {
+                    used_ids: &ids_from_closure,
+                });
+            }
+
             let new_arrow = ArrowExpr {
                 span: DUMMY_SP,
                 params: vec![
@@ -239,11 +245,13 @@ impl<C: Comments> ServerActions<C> {
             };
 
             // export const $ACTION_myAction = async () => {}
-            let mut new_params: Vec<Pat> = ids_from_closure
-                .iter()
-                .cloned()
-                .map(|id| Pat::Ident(Ident::from(id.0).into()))
-                .collect();
+            let mut new_params: Vec<Pat> = vec![];
+
+            for i in 0..ids_from_closure.len() {
+                new_params.push(Pat::Ident(
+                    Ident::new(format!("$$ACTION_ARG_{}", i).into(), DUMMY_SP).into(),
+                ));
+            }
             for p in a.params.iter() {
                 new_params.push(p.clone());
             }
@@ -313,6 +321,10 @@ impl<C: Comments> ServerActions<C> {
                 Some(action_ident.clone()),
             );
 
+            f.body.visit_mut_with(&mut ClosureReplacer {
+                used_ids: &ids_from_closure,
+            });
+
             let new_fn = Function {
                 params: vec![
                     // ...args
@@ -343,11 +355,18 @@ impl<C: Comments> ServerActions<C> {
             };
 
             // export async function $ACTION_myAction () {}
-            let mut new_params: Vec<Param> = ids_from_closure
-                .iter()
-                .cloned()
-                .map(|id| Param::from(Pat::Ident(Ident::from(id.0).into())))
-                .collect();
+            let mut new_params: Vec<Param> = vec![];
+
+            // $$ACTION_ARG_{index}
+            for i in 0..ids_from_closure.len() {
+                new_params.push(Param {
+                    span: DUMMY_SP,
+                    decorators: vec![],
+                    pat: Pat::Ident(
+                        Ident::new(format!("$$ACTION_ARG_{}", i).into(), DUMMY_SP).into(),
+                    ),
+                });
+            }
             for p in f.params.iter() {
                 new_params.push(p.clone());
             }
@@ -1432,6 +1451,32 @@ fn collect_idents_in_stmt(stmt: &Stmt) -> Vec<Id> {
     ids
 }
 
+pub(crate) struct ClosureReplacer<'a> {
+    used_ids: &'a [Name],
+}
+
+impl ClosureReplacer<'_> {
+    fn index(&self, e: &Expr) -> Option<usize> {
+        let name = Name::try_from(e).ok()?;
+        self.used_ids.iter().position(|used_id| *used_id == name)
+    }
+}
+
+impl VisitMut for ClosureReplacer<'_> {
+    fn visit_mut_expr(&mut self, e: &mut Expr) {
+        e.visit_mut_children_with(self);
+
+        if let Some(index) = self.index(e) {
+            *e = Expr::Ident(Ident::new(
+                // $$ACTION_ARG_0
+                format!("$$ACTION_ARG_{}", index).into(),
+                DUMMY_SP,
+            ));
+        }
+    }
+    noop_visit_mut_type!();
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 struct Name(Id, Vec<(JsWord, bool)>);
 

diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/1/input.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/1/input.js
@@ -8,3 +8,16 @@ export function Item({ id1, id2 }) {
   }
   return <Button action={deleteItem}>Delete</Button>
 }
+
+export default function Home() {
+  const info = {
+    name: 'John',
+    test: 'test',
+  }
+  const action = async () => {
+    'use server'
+    console.log(info.name)
+    console.log(info.test)
+  }
+  return null
+}
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/1/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/1/output.js
@@ -1,4 +1,4 @@
-/* __next_internal_action_entry_do_not_use__ $$ACTION_0 */ import __create_action_proxy__ from "private-next-rsc-action-proxy";
+/* __next_internal_action_entry_do_not_use__ $$ACTION_0,$$ACTION_2 */ import __create_action_proxy__ from "private-next-rsc-action-proxy";
 import deleteFromDb from 'db';
 export function Item({ id1 , id2  }) {
     async function deleteItem(...args) {
@@ -10,7 +10,23 @@ export function Item({ id1 , id2  }) {
     ], deleteItem, $$ACTION_0);
     return <Button action={deleteItem}>Delete</Button>;
 }
-export async function $$ACTION_0(id1, id2) {
-    await deleteFromDb(id1);
-    await deleteFromDb(id2);
+export async function $$ACTION_0($$ACTION_ARG_0, $$ACTION_ARG_1) {
+    await deleteFromDb($$ACTION_ARG_0);
+    await deleteFromDb($$ACTION_ARG_1);
 }
+export default function Home() {
+    const info = {
+        name: 'John',
+        test: 'test'
+    };
+    const action = ($$ACTION_1 = async (...args)=>$$ACTION_2.apply(null, ($$ACTION_1.$$bound || []).concat(args)), __create_action_proxy__("9878bfa39811ca7650992850a8751f9591b6a557", [
+        info.name,
+        info.test
+    ], $$ACTION_1, $$ACTION_2), $$ACTION_1);
+    return null;
+}
+export const $$ACTION_2 = async ($$ACTION_ARG_0, $$ACTION_ARG_1)=>{
+    console.log($$ACTION_ARG_0);
+    console.log($$ACTION_ARG_1);
+};
+var $$ACTION_1;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/16/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/16/output.js
@@ -9,10 +9,10 @@ export function Item({ id1 , id2  }) {
     ], $$ACTION_0, $$ACTION_1), $$ACTION_0);
     return <Button action={deleteItem}>Delete</Button>;
 }
-export const $$ACTION_1 = async (id1, v2)=>{
-    await deleteFromDb(id1);
+export const $$ACTION_1 = async ($$ACTION_ARG_0, $$ACTION_ARG_1)=>{
+    await deleteFromDb($$ACTION_ARG_0);
     await deleteFromDb(v1);
-    await deleteFromDb(v2);
+    await deleteFromDb($$ACTION_ARG_1);
 };
 var $$ACTION_0;
 const f = (x)=>{
@@ -23,15 +23,15 @@ const f = (x)=>{
         x
     ], g, $$ACTION_2);
 };
-export async function $$ACTION_2(x, y, ...z) {
-    return x + y + z[0];
+export async function $$ACTION_2($$ACTION_ARG_0, y, ...z) {
+    return $$ACTION_ARG_0 + y + z[0];
 }
 const g = (x)=>{
     f = ($$ACTION_3 = async (...args)=>$$ACTION_4.apply(null, ($$ACTION_3.$$bound || []).concat(args)), __create_action_proxy__("9c0dd1f7c2b3f41d32e10f5c437de3d67ad32c6c", [
         x
     ], $$ACTION_3, $$ACTION_4), $$ACTION_3);
 };
-export const $$ACTION_4 = async (x, y, ...z)=>{
-    return x + y + z[0];
+export const $$ACTION_4 = async ($$ACTION_ARG_0, y, ...z)=>{
+    return $$ACTION_ARG_0 + y + z[0];
 };
-var $$ACTION_3;
+var $$ACTION_3;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/18/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/18/output.js
@@ -27,15 +27,15 @@ export function Item({ id1 , id2  }) {
 
     </>;
 }
-export const $$ACTION_1 = async (id1, v2)=>{
-    await deleteFromDb(id1);
+export const $$ACTION_1 = async ($$ACTION_ARG_0, $$ACTION_ARG_1)=>{
+    await deleteFromDb($$ACTION_ARG_0);
     await deleteFromDb(v1);
-    await deleteFromDb(v2);
+    await deleteFromDb($$ACTION_ARG_1);
 };
 var $$ACTION_0;
-export async function $$ACTION_3(id1, v2) {
-    await deleteFromDb(id1);
+export async function $$ACTION_3($$ACTION_ARG_0, $$ACTION_ARG_1) {
+    await deleteFromDb($$ACTION_ARG_0);
     await deleteFromDb(v1);
-    await deleteFromDb(v2);
+    await deleteFromDb($$ACTION_ARG_1);
 }
 var $$ACTION_2;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/19/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/19/output.js
@@ -12,7 +12,7 @@ export function Item({ value  }) {
 
     </>;
 }
-export const $$ACTION_1 = async (value, value2)=>{
-    return value * value2;
+export const $$ACTION_1 = async ($$ACTION_ARG_0, value2)=>{
+    return $$ACTION_ARG_0 * value2;
 };
 var $$ACTION_0;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/21/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/21/output.js
@@ -9,8 +9,8 @@ export default function Page() {
         y
     ], $$ACTION_0, $$ACTION_1), $$ACTION_0))}/>;
 }
-export async function $$ACTION_1(y, z) {
-    return x + y + z;
+export async function $$ACTION_1($$ACTION_ARG_0, z) {
+    return x + $$ACTION_ARG_0 + z;
 }
 var $$ACTION_0;
 validator(($$ACTION_2 = async (...args)=>$$ACTION_3.apply(null, ($$ACTION_2.$$bound || []).concat(args)), __create_action_proxy__("56a859f462d35a297c46a1bbd1e6a9058c104ab8", null, $$ACTION_2, $$ACTION_3), $$ACTION_2));

diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/23/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/23/output.js
@@ -12,10 +12,10 @@ export default function Page({ foo , x , y  }) {
     ], $$ACTION_1, $$ACTION_2), $$ACTION_1);
     action2.bind(null, foo[0], foo[1], foo.x, foo[y]);
 }
-export async function $$ACTION_0(x, a, b, c, d) {
-    console.log(a, b, x, c, d);
+export async function $$ACTION_0($$ACTION_ARG_0, a, b, c, d) {
+    console.log(a, b, $$ACTION_ARG_0, c, d);
 }
-export const $$ACTION_2 = async (x, a, b, c, d)=>{
-    console.log(a, b, x, c, d);
+export const $$ACTION_2 = async ($$ACTION_ARG_0, a, b, c, d)=>{
+    console.log(a, b, $$ACTION_ARG_0, c, d);
 };
 var $$ACTION_1;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/5/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/5/output.js
@@ -12,8 +12,8 @@ export function Item({ id1 , id2  }) {
     ], deleteItem, $$ACTION_0);
     return <Button action={deleteItem}>Delete</Button>;
 }
-export async function $$ACTION_0(id1, v2) {
-    await deleteFromDb(id1);
+export async function $$ACTION_0($$ACTION_ARG_0, $$ACTION_ARG_1) {
+    await deleteFromDb($$ACTION_ARG_0);
     await deleteFromDb(v1);
-    await deleteFromDb(v2);
+    await deleteFromDb($$ACTION_ARG_1);
 }
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/6/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/6/output.js
@@ -31,11 +31,11 @@ export function y(p, [p1, { p2  }], ...p3) {
     ], action, $$ACTION_0);
     return <Button action={action}>Delete</Button>;
 }
-export async function $$ACTION_0(f2, f11, p, p1, p2, p3) {
+export async function $$ACTION_0($$ACTION_ARG_0, $$ACTION_ARG_1, $$ACTION_ARG_2, $$ACTION_ARG_3, $$ACTION_ARG_4, $$ACTION_ARG_5) {
     const f17 = 1;
     if (true) {
         const f18 = 1;
         const f19 = 1;
     }
-    console.log(f, f1, f2, f3, f4, f5, f6, f7, f8, f2(f9), f12, f11, f16.x, f17, f18, p, p1, p2, p3, g19, g20, globalThis);
+    console.log(f, f1, $$ACTION_ARG_0, f3, f4, f5, f6, f7, f8, $$ACTION_ARG_0(f9), f12, $$ACTION_ARG_1, f16.x, f17, f18, $$ACTION_ARG_2, $$ACTION_ARG_3, $$ACTION_ARG_4, $$ACTION_ARG_5, g19, g20, globalThis);
 }
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/7/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/7/output.js
@@ -14,6 +14,6 @@ export function Item(product, foo, bar) {
     ], deleteItem, $$ACTION_0);
     return <Button action={deleteItem}>Delete</Button>;
 }
-export async function $$ACTION_0(product, product, product, product, foo, bar) {
-    await deleteFromDb(product.id, product?.foo, product.bar.baz, product[foo, bar]);
+export async function $$ACTION_0($$ACTION_ARG_0, $$ACTION_ARG_1, $$ACTION_ARG_2, $$ACTION_ARG_3, $$ACTION_ARG_4, $$ACTION_ARG_5) {
+    await deleteFromDb($$ACTION_ARG_3.id, $$ACTION_ARG_3?.foo, $$ACTION_ARG_3.bar.baz, $$ACTION_ARG_3[$$ACTION_ARG_4, $$ACTION_ARG_5]);
 }
diff --git a/test/e2e/app-dir/actions/app/server/page.js b/test/e2e/app-dir/actions/app/server/page.js
@@ -4,15 +4,15 @@ import Form from './form'
 import dec, { inc } from './actions'
 
 export default function Page() {
-  const two = 2
+  const two = { value: 2 }
   return (
     <>
       <Counter
         inc={inc}
         dec={dec}
         double={async (x) => {
           'use server'
-          return x * two
+          return x * two.value
         }}
       />
       <Form />