Support passing both closure arguments and parameters (#47212)

When calling the server (via `callServer`), we concat all closure values (`$$bound`) and arguments of the function call into one array on the client. Hence on the server, we will have to compile the function differently to support that. With this change, the compiled function will have a `$$with_bound` flag to indicate that if it accepts closure values. If so, the only argument passed will be an array like `[...bound_values, ...fn_args]`, and we compile the function parameters to `(closure, arg1 = closure[N], arg2 = closure[N + 1], ...)` where `N` is the number of the closure identifiers. This way we can still fill these arguments by only pass an "bound + args" array. If it doesn't accept closure values, it will be directly called with `...fn_args` so no compilation change needed. The reason that we use `arg1 = closure[N]` is that this can support complex patterns in parameters such as `f(closure, {a} = closure[1], [b] = closure[2])`. fix NEXT-487 ([link](https://linear.app/vercel/issue/NEXT-487))
vercel · Mar 17, 2023 · 922b5de · 922b5de
1 parent 8dabe4f
commit 922b5de
Show file tree

Hide file tree

Showing 26 changed files with 112 additions and 18 deletions.
diff --git a/packages/next-swc/crates/core/src/server_actions.rs b/packages/next-swc/crates/core/src/server_actions.rs
@@ -147,6 +147,7 @@ impl<C: Comments> ServerActions<C> {
                 Vec::new(),
                 self.file_name.to_string(),
                 export_name.to_string(),
+                false,
             );
 
             // export const $ACTION_myAction = myAction;
@@ -203,6 +204,7 @@ impl<C: Comments> ServerActions<C> {
                         .collect(),
                     self.file_name.to_string(),
                     export_name.to_string(),
+                    true,
                 );
 
                 if let BlockStmtOrExpr::BlockStmt(block) = &mut *a.body {
@@ -223,6 +225,22 @@ impl<C: Comments> ServerActions<C> {
                 };
 
                 // export const $ACTION_myAction = async () => {}
+                let mut new_params: Vec<Pat> = vec![closure_arg.clone().into()];
+                for (i, p) in a.params.iter().enumerate() {
+                    new_params.push(Pat::Assign(AssignPat {
+                        span: DUMMY_SP,
+                        left: Box::new(p.clone()),
+                        right: Box::new(Expr::Member(MemberExpr {
+                            span: DUMMY_SP,
+                            obj: Box::new(Expr::Ident(closure_arg.clone())),
+                            prop: MemberProp::Computed(ComputedPropName {
+                                span: DUMMY_SP,
+                                expr: Box::new(Expr::from(ids_from_closure.len() + i)),
+                            }),
+                        })),
+                        type_ann: None,
+                    }));
+                }
                 self.extra_items
                     .push(ModuleItem::ModuleDecl(ModuleDecl::ExportDecl(ExportDecl {
                         span: DUMMY_SP,
@@ -234,7 +252,7 @@ impl<C: Comments> ServerActions<C> {
                                 span: DUMMY_SP,
                                 name: action_ident.into(),
                                 init: Some(Box::new(Expr::Arrow(ArrowExpr {
-                                    params: vec![closure_arg.into()],
+                                    params: new_params,
                                     ..a.clone()
                                 }))),
                                 definite: Default::default(),
@@ -285,6 +303,7 @@ impl<C: Comments> ServerActions<C> {
                         .collect(),
                     self.file_name.to_string(),
                     export_name.to_string(),
+                    true,
                 );
 
                 f.body.visit_mut_with(&mut ClosureReplacer {
@@ -310,13 +329,29 @@ impl<C: Comments> ServerActions<C> {
                 };
 
                 // export async function $ACTION_myAction () {}
+                let mut new_params: Vec<Param> = vec![closure_arg.clone().into()];
+                for (i, p) in f.params.iter().enumerate() {
+                    new_params.push(Param::from(Pat::Assign(AssignPat {
+                        span: DUMMY_SP,
+                        left: Box::new(p.pat.clone()),
+                        right: Box::new(Expr::Member(MemberExpr {
+                            span: DUMMY_SP,
+                            obj: Box::new(Expr::Ident(closure_arg.clone())),
+                            prop: MemberProp::Computed(ComputedPropName {
+                                span: DUMMY_SP,
+                                expr: Box::new(Expr::from(ids_from_closure.len() + i)),
+                            }),
+                        })),
+                        type_ann: None,
+                    })));
+                }
                 self.extra_items
                     .push(ModuleItem::ModuleDecl(ModuleDecl::ExportDecl(ExportDecl {
                         span: DUMMY_SP,
                         decl: FnDecl {
                             ident: action_ident,
                             function: Box::new(Function {
-                                params: vec![closure_arg.into()],
+                                params: new_params,
                                 ..*f.take()
                             }),
                             declare: Default::default(),
@@ -821,6 +856,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
                     Vec::new(),
                     self.file_name.to_string(),
                     export_name.to_string(),
+                    false,
                 );
                 if !self.config.is_server {
                     let params_ident = private_ident!("args");
@@ -993,6 +1029,7 @@ fn annotate_ident_as_action(
     bound: Vec<Option<ExprOrSpread>>,
     file_name: String,
     export_name: String,
+    has_bound: bool,
 ) {
     // myAction.$$typeof = Symbol.for('react.server.reference');
     annotations.push(annotate(
@@ -1030,6 +1067,12 @@ fn annotate_ident_as_action(
         }
         .into(),
     ));
+
+    // If an action doesn't have any bound values, we add a special property
+    // to mark that all parameters are just passed through.
+    if !has_bound {
+        annotations.push(annotate(&ident, "$$with_bound", Lit::from(false).into()));
+    }
 }
 
 const DIRECTIVE_TYPOS: &[&str] = &[

diff --git a/packages/next-swc/crates/core/tests/errors/server-actions/1/output.js b/packages/next-swc/crates/core/tests/errors/server-actions/1/output.js
@@ -2,3 +2,4 @@
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "ab21efdafbe611287bc25c0462b1e0510d13e48b";
 foo.$$bound = [];
+foo.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/errors/server-actions/2/output.js b/packages/next-swc/crates/core/tests/errors/server-actions/2/output.js
@@ -3,3 +3,4 @@ export function bar() {}
 bar.$$typeof = Symbol.for("react.server.reference");
 bar.$$id = "ac840dcaf5e8197cb02b7f3a43c119b7a770b272";
 bar.$$bound = [];
+bar.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/errors/server-actions/3/output.js b/packages/next-swc/crates/core/tests/errors/server-actions/3/output.js
@@ -2,3 +2,4 @@
 x.$$typeof = Symbol.for("react.server.reference");
 x.$$id = "b78c261f135a7a852508c2920bd7228020ff4bd7";
 x.$$bound = [];
+x.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/client/1/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/client/1/output.js
@@ -8,6 +8,8 @@ export default async function $$ACTION_0(...args) {
 myAction.$$typeof = Symbol.for("react.server.reference");
 myAction.$$id = "e10665baac148856374b2789aceb970f66fec33e";
 myAction.$$bound = [];
+myAction.$$with_bound = false;
 $$ACTION_0.$$typeof = Symbol.for("react.server.reference");
 $$ACTION_0.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
 $$ACTION_0.$$bound = [];
+$$ACTION_0.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/client/2/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/client/2/output.js
@@ -5,3 +5,4 @@
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "ab21efdafbe611287bc25c0462b1e0510d13e48b";
 foo.$$bound = [];
+foo.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/10/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/10/output.js
@@ -1,4 +1,5 @@
 /* __next_internal_action_entry_do_not_use__ default */ export default async function foo() {}
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
-foo.$$bound = [];
+foo.$$bound = [];
+foo.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/11/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/11/output.js
@@ -2,3 +2,4 @@
 $$ACTION_0.$$typeof = Symbol.for("react.server.reference");
 $$ACTION_0.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
 $$ACTION_0.$$bound = [];
+$$ACTION_0.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/12/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/12/output.js
@@ -3,3 +3,4 @@ export default foo;
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
 foo.$$bound = [];
+foo.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/13/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/13/output.js
@@ -5,6 +5,8 @@ export { bar };
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
 foo.$$bound = [];
+foo.$$with_bound = false;
 bar.$$typeof = Symbol.for("react.server.reference");
 bar.$$id = "ac840dcaf5e8197cb02b7f3a43c119b7a770b272";
 bar.$$bound = [];
+bar.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/14/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/14/output.js
@@ -4,3 +4,4 @@
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "ab21efdafbe611287bc25c0462b1e0510d13e48b";
 foo.$$bound = [];
+foo.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/15/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/15/output.js
@@ -5,3 +5,4 @@ var $$ACTION_0;
 $$ACTION_0.$$typeof = Symbol.for("react.server.reference");
 $$ACTION_0.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
 $$ACTION_0.$$bound = [];
+$$ACTION_0.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/17/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/17/output.js
@@ -4,6 +4,8 @@ export { bar };
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "ab21efdafbe611287bc25c0462b1e0510d13e48b";
 foo.$$bound = [];
+foo.$$with_bound = false;
 bar.$$typeof = Symbol.for("react.server.reference");
 bar.$$id = "ac840dcaf5e8197cb02b7f3a43c119b7a770b272";
-bar.$$bound = [];
+bar.$$bound = [];
+bar.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/18/input.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/18/input.js
@@ -4,8 +4,6 @@ const v1 = 'v1';
 
 export function Item({ id1, id2 }) {
     const v2 = id2;
-
-    // TODO: Fix the inline function cases.
     return <>
         <Button action={async () => {
             "use server";

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
@@ -2,7 +2,6 @@
 const v1 = 'v1';
 export function Item({ id1 , id2  }) {
     const v2 = id2;
-    // TODO: Fix the inline function cases.
     return <>
 
         <Button action={$$ACTION_0 = async ()=>$$ACTION_1($$ACTION_0.$$bound), $$ACTION_0.$$typeof = Symbol.for("react.server.reference"), $$ACTION_0.$$id = "188d5d945750dc32e2c842b93c75a65763d4a922", $$ACTION_0.$$bound = [

diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/19/input.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/19/input.js
@@ -0,0 +1,8 @@
+export function Item({ value }) {
+    return <>
+        <Button action={async (value2) => {
+            "use server";
+            return value * value2;
+        }}>Multiple</Button>
+    </>
+}
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
@@ -0,0 +1,13 @@
+/* __next_internal_action_entry_do_not_use__ $$ACTION_1 */ export function Item({ value  }) {
+    return <>
+
+        <Button action={$$ACTION_0 = async (value2)=>$$ACTION_1($$ACTION_0.$$bound), $$ACTION_0.$$typeof = Symbol.for("react.server.reference"), $$ACTION_0.$$id = "188d5d945750dc32e2c842b93c75a65763d4a922", $$ACTION_0.$$bound = [
+        value
+    ], $$ACTION_0}>Multiple</Button>
+
+    </>;
+}
+export const $$ACTION_1 = async (closure, value2 = closure[1])=>{
+    return closure[0] * value2;
+};
+var $$ACTION_0;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/2/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/2/output.js
@@ -4,6 +4,7 @@
 myAction.$$typeof = Symbol.for("react.server.reference");
 myAction.$$id = "6d53ce510b2e36499b8f56038817b9bad86cabb4";
 myAction.$$bound = [];
+myAction.$$with_bound = false;
 export const $$ACTION_0 = myAction;
 export default function Page() {
     return <Button action={myAction}>Delete</Button>;

diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/3/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/3/output.js
@@ -5,3 +5,4 @@
 myAction.$$typeof = Symbol.for("react.server.reference");
 myAction.$$id = "e10665baac148856374b2789aceb970f66fec33e";
 myAction.$$bound = [];
+myAction.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/4/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/4/output.js
@@ -8,9 +8,12 @@ function Foo() {
 a.$$typeof = Symbol.for("react.server.reference");
 a.$$id = "6e7bc104e4d6e7fda190c4a51be969cfd0be6d6d";
 a.$$bound = [];
+a.$$with_bound = false;
 b.$$typeof = Symbol.for("react.server.reference");
 b.$$id = "d1f7eb64271d7c601dfef7d4d7053de1c2ca4338";
 b.$$bound = [];
+b.$$with_bound = false;
 c.$$typeof = Symbol.for("react.server.reference");
 c.$$id = "1ab723c80dcca470e0410b4b2a2fc2bf21f41476";
 c.$$bound = [];
+c.$$with_bound = false;
diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/8/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/8/output.js
@@ -6,6 +6,7 @@
 myAction.$$typeof = Symbol.for("react.server.reference");
 myAction.$$id = "6d53ce510b2e36499b8f56038817b9bad86cabb4";
 myAction.$$bound = [];
+myAction.$$with_bound = false;
 export const $$ACTION_0 = myAction;
 export default function Page() {
     return <Button action={myAction}>Delete</Button>;

diff --git a/packages/next-swc/crates/core/tests/fixture/server-actions/server/9/output.js b/packages/next-swc/crates/core/tests/fixture/server-actions/server/9/output.js
@@ -8,9 +8,12 @@ export { qux as default };
 foo.$$typeof = Symbol.for("react.server.reference");
 foo.$$id = "ab21efdafbe611287bc25c0462b1e0510d13e48b";
 foo.$$bound = [];
+foo.$$with_bound = false;
 bar.$$typeof = Symbol.for("react.server.reference");
 bar.$$id = "050e3854b72b19e3c7e3966a67535543a90bf7e0";
 bar.$$bound = [];
+bar.$$with_bound = false;
 qux.$$typeof = Symbol.for("react.server.reference");
 qux.$$id = "c18c215a6b7cdc64bf709f3a714ffdef1bf9651d";
 qux.$$bound = [];
+qux.$$with_bound = false;
diff --git a/packages/next/src/build/webpack/loaders/next-flight-action-entry-loader.ts b/packages/next/src/build/webpack/loaders/next-flight-action-entry-loader.ts
@@ -28,9 +28,14 @@ ${actionList
   .join('\n')}
 }
 
-async function endpoint(id, bound) {
+async function endpoint(id, args) {
   const action = await actions[id]()
-  return action.apply(null, bound)
+  
+  if (action.$$with_bound === false) {
+    return action.apply(null, args)
+  }
+
+  return action.call(null, args)
 }
 
 // Using "export default" will cause this to be tree-shaken away due to unused exports.

diff --git a/packages/next/src/client/app-call-server.ts b/packages/next/src/client/app-call-server.ts
@@ -1,4 +1,4 @@
-export async function callServer(id: string, args: any[]) {
+export async function callServer(id: string, bound: any[]) {
   const actionId = id
 
   // Fetching the current url with the action header.
@@ -10,7 +10,7 @@ export async function callServer(id: string, args: any[]) {
       'Next-Action': actionId,
     },
     body: JSON.stringify({
-      bound: args,
+      bound,
     }),
   })
 

diff --git a/test/e2e/app-dir/actions/app/server/actions.js b/test/e2e/app-dir/actions/app/server/actions.js
@@ -4,10 +4,6 @@ export async function inc(value) {
   return value + 1
 }
 
-export async function dec(value) {
+export default async function dec(value) {
   return value - 1
 }
-
-export default async function (value) {
-  return value * 2
-}
diff --git a/test/e2e/app-dir/actions/app/server/page.js b/test/e2e/app-dir/actions/app/server/page.js
@@ -1,12 +1,20 @@
 import Counter from './counter'
 import Form from './form'
 
-import double, { inc, dec } from './actions'
+import dec, { inc } from './actions'
 
 export default function Page() {
+  const two = 2
   return (
     <>
-      <Counter inc={inc} dec={dec} double={double} />
+      <Counter
+        inc={inc}
+        dec={dec}
+        double={async (x) => {
+          'use server'
+          return x * two
+        }}
+      />
       <Form />
     </>
   )