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.
vercel · Nov 21, 2024 · 03ea43b · 03ea43b
1 parent fb9b9d7
commit 03ea43b
Show file tree

Hide file tree

Showing 13 changed files with 262 additions and 0 deletions.
diff --git 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,
     },
@@ -1419,6 +1425,68 @@ impl<C: Comments> VisitMut for ServerActions<C> {
         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.clone();
+            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_callee(&mut self, n: &mut Callee) {
         let old_in_callee = self.in_callee;
         self.in_callee = true;
@@ -2837,6 +2905,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! {
@@ -2846,6 +2923,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,18 @@
+'use client'
+
+import { useActionState } from 'react'
+
+export function Form({
+  getRandomValue,
+}: {
+  getRandomValue: () => Promise<number>
+}) {
+  const [result, formAction, isPending] = useActionState(getRandomValue, -1)
+
+  return (
+    <form action={formAction}>
+      <button>Submit</button>
+      <p>{isPending ? 'loading...' : result}</p>
+    </form>
+  )
+}
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
@@ -0,0 +1,6 @@
+import { Cached } from './cached'
+import { Form } from './form'
+
+export default function Page() {
+  return <Form getRandomValue={Cached.getRandomValue} />
+}
diff --git 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)
+    })
+  })
 })