From 18d2f7da33145c359724153b018e8ab943e65613 Mon Sep 17 00:00:00 2001
From: Shu Ding <g@shud.in>
Date: Thu, 8 Jun 2023 05:03:54 +0200
Subject: [PATCH] Fix shorthand prop wrongly handled in the Server Actions
 compiler (#50937)

This PR fixes the case of having a shorthand prop `{ id }` referenced from a non-top level closure in the Server Actions compiler. The main problem is that an `Ident` in `PropOrSpread` is not considered as an `Expr`.

See corresponding issue report and test case for more details.
fix #50445
fix NEXT-1254
---
 .../crates/core/src/server_actions.rs         | 41 ++++++++++++++++++-
 .../fixture/server-actions/server/5/input.js  |  4 +-
 .../fixture/server-actions/server/5/output.js | 12 ++++--
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/packages/next-swc/crates/core/src/server_actions.rs b/packages/next-swc/crates/core/src/server_actions.rs
index c67fa721361..6ef26197d56 100644
--- a/packages/next-swc/crates/core/src/server_actions.rs
+++ b/packages/next-swc/crates/core/src/server_actions.rs
@@ -357,7 +357,7 @@ impl<C: Comments> ServerActions<C> {
             // export async function $ACTION_myAction () {}
             let mut new_params: Vec<Param> = vec![];
 
-            // $$ACTION_ARG_{index}
+            // add params from closure collected ids
             for i in 0..ids_from_closure.len() {
                 new_params.push(Param {
                     span: DUMMY_SP,
@@ -618,6 +618,20 @@ impl<C: Comments> VisitMut for ServerActions<C> {
         }
     }
 
+    fn visit_mut_prop_or_spread(&mut self, n: &mut PropOrSpread) {
+        if self.in_action_fn && self.in_action_closure {
+            if let PropOrSpread::Prop(box Prop::Shorthand(i)) = n {
+                self.in_action_closure = false;
+                self.action_closure_idents.push(Name::from(&*i));
+                n.visit_mut_children_with(self);
+                self.in_action_closure = true;
+                return;
+            }
+        }
+
+        n.visit_mut_children_with(self);
+    }
+
     fn visit_mut_expr(&mut self, n: &mut Expr) {
         if self.in_action_fn && self.in_action_closure {
             if let Ok(name) = Name::try_from(&*n) {
@@ -1474,12 +1488,37 @@ impl VisitMut for ClosureReplacer<'_> {
             ));
         }
     }
+
+    fn visit_mut_prop_or_spread(&mut self, n: &mut PropOrSpread) {
+        n.visit_mut_children_with(self);
+
+        if let PropOrSpread::Prop(box Prop::Shorthand(i)) = n {
+            let name = Name::from(&*i);
+            if let Some(index) = self.used_ids.iter().position(|used_id| *used_id == name) {
+                *n = PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp {
+                    key: PropName::Ident(i.clone()),
+                    value: Box::new(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)>);
 
+impl From<&'_ Ident> for Name {
+    fn from(value: &Ident) -> Self {
+        Name(value.to_id(), vec![])
+    }
+}
+
 impl TryFrom<&'_ Expr> for Name {
     type Error = ();
 
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/5/input.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/5/input.js
index e110568a3b9..eaa5cee624e 100644
--- a/packages/next-swc/crates/core/tests/fixture/server-actions/server/5/input.js
+++ b/packages/next-swc/crates/core/tests/fixture/server-actions/server/5/input.js
@@ -2,13 +2,15 @@ import deleteFromDb from 'db'
 
 const v1 = 'v1'
 
-export function Item({ id1, id2 }) {
+export function Item({ id1, id2, id3, id4 }) {
   const v2 = id2
   async function deleteItem() {
     'use server'
     await deleteFromDb(id1)
     await deleteFromDb(v1)
     await deleteFromDb(v2)
+    await deleteFromDb({ id3 })
+    await deleteFromDb(id4.x)
   }
   return <Button action={deleteItem}>Delete</Button>
 }
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
index c4c85c06966..f6341e495b6 100644
--- 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
@@ -1,19 +1,25 @@
 /* __next_internal_action_entry_do_not_use__ $$ACTION_0 */ import __create_action_proxy__ from "private-next-rsc-action-proxy";
 import deleteFromDb from 'db';
 const v1 = 'v1';
-export function Item({ id1 , id2  }) {
+export function Item({ id1 , id2 , id3 , id4  }) {
     const v2 = id2;
     async function deleteItem(...args) {
         return $$ACTION_0.apply(null, (deleteItem.$$bound || []).concat(args));
     }
     __create_action_proxy__("6d53ce510b2e36499b8f56038817b9bad86cabb4", [
         id1,
-        v2
+        v2,
+        id3,
+        id4.x
     ], deleteItem, $$ACTION_0);
     return <Button action={deleteItem}>Delete</Button>;
 }
-export async function $$ACTION_0($$ACTION_ARG_0, $$ACTION_ARG_1) {
+export async function $$ACTION_0($$ACTION_ARG_0, $$ACTION_ARG_1, $$ACTION_ARG_2, $$ACTION_ARG_3) {
     await deleteFromDb($$ACTION_ARG_0);
     await deleteFromDb(v1);
     await deleteFromDb($$ACTION_ARG_1);
+    await deleteFromDb({
+        id3: $$ACTION_ARG_2
+    });
+    await deleteFromDb($$ACTION_ARG_3);
 }