From b52b5cc7f1b20110fee72351861c34cfe4df7426 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 21 Nov 2024 18:50:38 +0100 Subject: [PATCH] Handle server function directives in class methods With this PR, we are allowing _static_ class methods to be annotated with `"use cache"` or `"use server"`. Class _instance_ methods on the other hand are not allowed as server functions. --- .../src/transforms/server_actions.rs | 86 +++++++++++++++++++ .../server-actions/server-graph/25/input.js | 12 +++ .../server-actions/server-graph/25/output.js | 10 +++ .../server-graph/25/output.stderr | 24 ++++++ .../server-actions/server-graph/26/input.js | 13 +++ .../server-actions/server-graph/26/output.js | 19 ++++ .../server-graph/26/output.stderr | 16 ++++ .../fixture/server-actions/server/57/input.js | 12 +++ .../server-actions/server/57/output.js | 17 ++++ .../app/static-class-method/cached.ts | 8 ++ .../app/static-class-method/form.tsx | 18 ++++ .../app/static-class-method/page.tsx | 6 ++ test/e2e/app-dir/use-cache/use-cache.test.ts | 21 +++++ 13 files changed, 262 insertions(+) create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.stderr create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.stderr create mode 100644 crates/next-custom-transforms/tests/fixture/server-actions/server/57/input.js create mode 100644 crates/next-custom-transforms/tests/fixture/server-actions/server/57/output.js create mode 100644 test/e2e/app-dir/use-cache/app/static-class-method/cached.ts create mode 100644 test/e2e/app-dir/use-cache/app/static-class-method/form.tsx create mode 100644 test/e2e/app-dir/use-cache/app/static-class-method/page.tsx diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 1468cb951e532..48e937567aa18 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -67,9 +67,15 @@ enum ServerActionsErrorKind { span: Span, directive: Directive, }, + InlineUseCacheInClassInstanceMethod { + span: Span, + }, InlineUseCacheInClientComponent { span: Span, }, + InlineUseServerInClassInstanceMethod { + span: Span, + }, InlineUseServerInClientComponent { span: Span, }, @@ -1260,6 +1266,68 @@ impl VisitMut for ServerActions { self.in_exported_expr = old_in_exported_expr; } + fn visit_mut_class_member(&mut self, n: &mut ClassMember) { + if let ClassMember::Method(ClassMethod { + is_abstract: false, + is_static: true, + kind: MethodKind::Method, + key, + span, + accessibility: None | Some(Accessibility::Public), + .. + }) = n + { + let key = key.clone(); + let span = *span; + let old_arrow_or_fn_expr_ident = self.arrow_or_fn_expr_ident.clone(); + + if let PropName::Ident(ident_name) = &key { + self.arrow_or_fn_expr_ident = Some(ident_name.clone().into()); + } + + let old_this_status = self.this_status.clone(); + self.this_status = ThisStatus::Allowed; + self.rewrite_expr_to_proxy_expr = None; + self.in_exported_expr = false; + n.visit_mut_children_with(self); + self.this_status = old_this_status; + self.arrow_or_fn_expr_ident = old_arrow_or_fn_expr_ident; + + if let Some(expr) = &self.rewrite_expr_to_proxy_expr { + *n = ClassMember::ClassProp(ClassProp { + span, + key, + value: Some(expr.clone()), + is_static: true, + ..Default::default() + }); + self.rewrite_expr_to_proxy_expr = None; + } + } else { + n.visit_mut_children_with(self); + } + } + + fn visit_mut_class_method(&mut self, n: &mut ClassMethod) { + if n.is_static { + n.visit_mut_children_with(self); + } else { + let (is_action_fn, is_cache_fn) = has_body_directive(&n.function.body); + + if is_action_fn { + emit_error( + ServerActionsErrorKind::InlineUseServerInClassInstanceMethod { span: n.span }, + ); + } + + if is_cache_fn { + emit_error( + ServerActionsErrorKind::InlineUseCacheInClassInstanceMethod { span: n.span }, + ); + } + } + } + fn visit_mut_call_expr(&mut self, n: &mut CallExpr) { if let Callee::Expr(box Expr::Ident(Ident { sym, .. })) = &mut n.callee { if sym == "jsxDEV" || sym == "_jsxDEV" { @@ -2850,6 +2918,15 @@ fn emit_error(error_kind: ServerActionsErrorKind) { } }, ), + ServerActionsErrorKind::InlineUseCacheInClassInstanceMethod { span } => ( + span, + formatdoc! { + r#" + It is not allowed to define inline "use cache" annotated class instance methods. + To define cached functions, use functions, object method properties, or static class methods instead. + "# + }, + ), ServerActionsErrorKind::InlineUseCacheInClientComponent { span } => ( span, formatdoc! { @@ -2859,6 +2936,15 @@ fn emit_error(error_kind: ServerActionsErrorKind) { "# }, ), + ServerActionsErrorKind::InlineUseServerInClassInstanceMethod { span } => ( + span, + formatdoc! { + r#" + It is not allowed to define inline "use server" annotated class instance methods. + To define Server Actions, use functions, object method properties, or static class methods instead. + "# + }, + ), ServerActionsErrorKind::InlineUseServerInClientComponent { span } => ( span, formatdoc! { diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/input.js new file mode 100644 index 0000000000000..c818c2bf03a53 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/input.js @@ -0,0 +1,12 @@ +export class MyClass { + async foo() { + 'use cache' + + return fetch('https://example.com').then((res) => res.json()) + } + async bar() { + 'use server' + + console.log(42) + } +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.js new file mode 100644 index 0000000000000..477db72fc14ad --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.js @@ -0,0 +1,10 @@ +export class MyClass { + async foo() { + 'use cache'; + return fetch('https://example.com').then((res)=>res.json()); + } + async bar() { + 'use server'; + console.log(42); + } +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.stderr new file mode 100644 index 0000000000000..542bea78f37df --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/25/output.stderr @@ -0,0 +1,24 @@ + x It is not allowed to define inline "use cache" annotated class instance methods. + | To define cached functions, use functions, object method properties, or static class methods instead. + | + ,-[input.js:2:1] + 1 | export class MyClass { + 2 | ,-> async foo() { + 3 | | 'use cache' + 4 | | + 5 | | return fetch('https://example.com').then((res) => res.json()) + 6 | `-> } + 7 | async bar() { + `---- + x It is not allowed to define inline "use server" annotated class instance methods. + | To define Server Actions, use functions, object method properties, or static class methods instead. + | + ,-[input.js:7:1] + 6 | } + 7 | ,-> async bar() { + 8 | | 'use server' + 9 | | + 10 | | console.log(42) + 11 | `-> } + 12 | } + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/input.js new file mode 100644 index 0000000000000..6996bf0752c93 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/input.js @@ -0,0 +1,13 @@ +export class MyClass { + static async foo() { + return fetch('https://example.com').then((res) => res.json()) + } + static async bar() { + 'use cache' + + // arguments is not allowed here + console.log(arguments) + // this is not allowed here + return this.foo() + } +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.js new file mode 100644 index 0000000000000..bc52378465ff1 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.js @@ -0,0 +1,19 @@ +/* __next_internal_action_entry_do_not_use__ {"803128060c414d59f8552e4788b846c0d2b7f74743":"$$RSC_SERVER_CACHE_0"} */ import { registerServerReference } from "private-next-rsc-server-reference"; +import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption"; +import { cache as $$cache__ } from "private-next-rsc-cache-wrapper"; +export var $$RSC_SERVER_CACHE_0 = $$cache__("default", "803128060c414d59f8552e4788b846c0d2b7f74743", 0, /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ async function bar() { + // arguments is not allowed here + console.log(arguments); + // this is not allowed here + return this.foo(); +}); +Object.defineProperty($$RSC_SERVER_CACHE_0, "name", { + "value": "bar", + "writable": false +}); +export class MyClass { + static async foo() { + return fetch('https://example.com').then((res)=>res.json()); + } + static bar = registerServerReference($$RSC_SERVER_CACHE_0, "803128060c414d59f8552e4788b846c0d2b7f74743", null); +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.stderr new file mode 100644 index 0000000000000..d78e16e543aa4 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/26/output.stderr @@ -0,0 +1,16 @@ + x "use cache" functions can not use `arguments`. + | + ,-[input.js:9:1] + 8 | // arguments is not allowed here + 9 | console.log(arguments) + : ^^^^^^^^^ + 10 | // this is not allowed here + `---- + x "use cache" functions can not use `this`. + | + ,-[input.js:11:1] + 10 | // this is not allowed here + 11 | return this.foo() + : ^^^^ + 12 | } + `---- diff --git a/crates/next-custom-transforms/tests/fixture/server-actions/server/57/input.js b/crates/next-custom-transforms/tests/fixture/server-actions/server/57/input.js new file mode 100644 index 0000000000000..2d588a600d449 --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/server-actions/server/57/input.js @@ -0,0 +1,12 @@ +export class MyClass { + static async foo() { + 'use cache' + + return fetch('https://example.com').then((res) => res.json()) + } + static async bar() { + 'use server' + + console.log(42) + } +} diff --git a/crates/next-custom-transforms/tests/fixture/server-actions/server/57/output.js b/crates/next-custom-transforms/tests/fixture/server-actions/server/57/output.js new file mode 100644 index 0000000000000..175231a3d2b04 --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/server-actions/server/57/output.js @@ -0,0 +1,17 @@ +/* __next_internal_action_entry_do_not_use__ {"0090b5db271335765a4b0eab01f044b381b5ebd5cd":"$$RSC_SERVER_ACTION_1","803128060c414d59f8552e4788b846c0d2b7f74743":"$$RSC_SERVER_CACHE_0"} */ import { registerServerReference } from "private-next-rsc-server-reference"; +import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption"; +import { cache as $$cache__ } from "private-next-rsc-cache-wrapper"; +export var $$RSC_SERVER_CACHE_0 = $$cache__("default", "803128060c414d59f8552e4788b846c0d2b7f74743", 0, /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ async function foo() { + return fetch('https://example.com').then((res)=>res.json()); +}); +Object.defineProperty($$RSC_SERVER_CACHE_0, "name", { + "value": "foo", + "writable": false +}); +export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ $$RSC_SERVER_ACTION_1 = async function bar() { + console.log(42); +}; +export class MyClass { + static foo = registerServerReference($$RSC_SERVER_CACHE_0, "803128060c414d59f8552e4788b846c0d2b7f74743", null); + static bar = registerServerReference($$RSC_SERVER_ACTION_1, "0090b5db271335765a4b0eab01f044b381b5ebd5cd", null); +} diff --git a/test/e2e/app-dir/use-cache/app/static-class-method/cached.ts b/test/e2e/app-dir/use-cache/app/static-class-method/cached.ts new file mode 100644 index 0000000000000..3d5c0aa004fd0 --- /dev/null +++ b/test/e2e/app-dir/use-cache/app/static-class-method/cached.ts @@ -0,0 +1,8 @@ +export class Cached { + static async getRandomValue() { + 'use cache' + const v = Math.random() + console.log(v) + return v + } +} diff --git a/test/e2e/app-dir/use-cache/app/static-class-method/form.tsx b/test/e2e/app-dir/use-cache/app/static-class-method/form.tsx new file mode 100644 index 0000000000000..bf3a36c2c803c --- /dev/null +++ b/test/e2e/app-dir/use-cache/app/static-class-method/form.tsx @@ -0,0 +1,18 @@ +'use client' + +import { useActionState } from 'react' + +export function Form({ + getRandomValue, +}: { + getRandomValue: () => Promise +}) { + const [result, formAction, isPending] = useActionState(getRandomValue, -1) + + return ( +
+ +

{isPending ? 'loading...' : result}

+
+ ) +} diff --git a/test/e2e/app-dir/use-cache/app/static-class-method/page.tsx b/test/e2e/app-dir/use-cache/app/static-class-method/page.tsx new file mode 100644 index 0000000000000..3023f48a95a4e --- /dev/null +++ b/test/e2e/app-dir/use-cache/app/static-class-method/page.tsx @@ -0,0 +1,6 @@ +import { Cached } from './cached' +import { Form } from './form' + +export default function Page() { + return
+} diff --git a/test/e2e/app-dir/use-cache/use-cache.test.ts b/test/e2e/app-dir/use-cache/use-cache.test.ts index cdafe76208da2..819f70f418302 100644 --- a/test/e2e/app-dir/use-cache/use-cache.test.ts +++ b/test/e2e/app-dir/use-cache/use-cache.test.ts @@ -431,4 +431,25 @@ describe('use-cache', () => { expect(await browser.elementByCss('#form-2 p').text()).toBe(value2) }) }) + + it('works with "use cache" in static class methods', async () => { + const browser = await next.browser('/static-class-method') + + let value = await browser.elementByCss('p').text() + + expect(value).toBe('-1') + + await browser.elementByCss('button').click() + + await retry(async () => { + value = await browser.elementByCss('p').text() + expect(value).toMatch(/\d\.\d+/) + }) + + await browser.elementByCss('button').click() + + await retry(async () => { + expect(await browser.elementByCss('p').text()).toBe(value) + }) + }) })