From 6fa146a45e8641fef1ee6a985651ba8748841a94 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Fri, 20 Dec 2024 15:11:23 +0100 Subject: [PATCH] Allow usage of `this` and `arguments` in nested function expression (#74179) In #73059 we added build-time checks for the forbidden usage of `this` and `arguments` in server functions. A nested function expression is however allowed to use these expressions. fixes #74181 To add a bit more context: `this` is forbidden in server actions because the Next.js compiler hoists inline server actions out of their original location into the module scope, so that they can be imported and invoked by the action handler. Due to this hoisting, the `this` context gets lost and is not available to the server action. To prevent surprising runtime errors for such cases, we emit a build error to provide feedback to developers as early as possible. However, nested function declarations or function expressions create a new `this` context, and in those it's allowed to access `this` and `arguments`. For consistency, and to prevent surprises when refactoring server actions, we apply the same rules for module-level server actions. --- .../src/transforms/server_actions.rs | 8 ++++ .../fixture/server-actions/server/59/input.js | 41 ++++++++++++++++++ .../server-actions/server/59/output.js | 42 +++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 crates/next-custom-transforms/tests/fixture/server-actions/server/59/input.js create mode 100644 crates/next-custom-transforms/tests/fixture/server-actions/server/59/output.js diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index a6d4a2c45293d..26548b6f569e2 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -932,11 +932,13 @@ impl VisitMut for ServerActions { } fn visit_mut_fn_expr(&mut self, f: &mut FnExpr) { + let old_this_status = replace(&mut self.this_status, ThisStatus::Allowed); let old_arrow_or_fn_expr_ident = self.arrow_or_fn_expr_ident.clone(); if let Some(ident) = &f.ident { self.arrow_or_fn_expr_ident = Some(ident.clone()); } f.visit_mut_children_with(self); + self.this_status = old_this_status; self.arrow_or_fn_expr_ident = old_arrow_or_fn_expr_ident; } @@ -1264,6 +1266,12 @@ impl VisitMut for ServerActions { self.in_exported_expr = old_in_exported_expr; } + fn visit_mut_class(&mut self, n: &mut Class) { + let old_this_status = replace(&mut self.this_status, ThisStatus::Allowed); + n.visit_mut_children_with(self); + self.this_status = old_this_status; + } + fn visit_mut_class_member(&mut self, n: &mut ClassMember) { if let ClassMember::Method(ClassMethod { is_abstract: false, diff --git a/crates/next-custom-transforms/tests/fixture/server-actions/server/59/input.js b/crates/next-custom-transforms/tests/fixture/server-actions/server/59/input.js new file mode 100644 index 0000000000000..d6748a1491c34 --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/server-actions/server/59/input.js @@ -0,0 +1,41 @@ +'use server' + +import { db } from './database' + +export const createItem = async (title) => { + return new Promise((resolve, reject) => { + db.serialize(() => { + db.run( + `INSERT INTO items (title) VALUES ($title)`, + { $title: title }, + function () { + // arguments is allowed here + const [err] = arguments + + if (err) { + reject(err) + } + + // this is allowed here + resolve(this.lastID) + } + ) + }) + }) +} + +export async function test() { + const MyClass = class { + x = 1 + foo() { + // this is allowed here + return this.x + } + bar = () => { + // this is allowed here + return this.x + } + } + const myObj = new MyClass() + return myObj.foo() + myObj.bar() +} diff --git a/crates/next-custom-transforms/tests/fixture/server-actions/server/59/output.js b/crates/next-custom-transforms/tests/fixture/server-actions/server/59/output.js new file mode 100644 index 0000000000000..763ce6dbed995 --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/server-actions/server/59/output.js @@ -0,0 +1,42 @@ +/* __next_internal_action_entry_do_not_use__ {"00b98e10c56c1bee7af6ff753c91a4c70ab0419f0a":"test","7f7a40999f9833d2bb66a82d6a20a7f2a7810315f8":"createItem"} */ import { registerServerReference } from "private-next-rsc-server-reference"; +import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption"; +import { db } from './database'; +export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ createItem = async (title)=>{ + return new Promise((resolve, reject)=>{ + db.serialize(()=>{ + db.run(`INSERT INTO items (title) VALUES ($title)`, { + $title: title + }, function() { + // arguments is allowed here + const [err] = arguments; + if (err) { + reject(err); + } + // this is allowed here + resolve(this.lastID); + }); + }); + }); +}; +export async function /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ test() { + const MyClass = class { + x = 1; + foo() { + // this is allowed here + return this.x; + } + bar = ()=>{ + // this is allowed here + return this.x; + }; + }; + const myObj = new MyClass(); + return myObj.foo() + myObj.bar(); +} +import { ensureServerEntryExports } from "private-next-rsc-action-validate"; +ensureServerEntryExports([ + createItem, + test +]); +registerServerReference(createItem, "7f7a40999f9833d2bb66a82d6a20a7f2a7810315f8", null); +registerServerReference(test, "00b98e10c56c1bee7af6ff753c91a4c70ab0419f0a", null);