From 04adb764057ce6fa82fd8b783585deadb00bbd63 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Fri, 22 Nov 2024 21:35:44 +0100 Subject: [PATCH] Disallow multiple server directives at the same level (file or function) (#73018) This PR has two main goals: - **Consolidate the detection of server directives in modules and function bodies.** Previously, these were two separate implementations with significant overlap, but also some [questionable discrepancies](https://github.com/vercel/next.js/pull/72811#discussion_r1843660381). - **Model the current directive (in a file or function) using an enum instead of two separate booleans.** This change prevents any confusion that both directives might be present simultaneously. Additionally, we're now emitting an error if multiple directives (`"use server"` and `"use cache"`) are defined in the same location (at the top of a file or function body). A mixed usage of `"use server"` and `"use cache"` _across different locations_ is still allowed, e.g. `"use cache"` at the top of a file, and `"use server"` in a function. > [!NOTE] > The diff may be tricky to review because chunks from two different functions are combined into a single function. Fortunately, we have comprehensive test coverage for the transforms, which instills high confidence that these changes are correct. --- .prettierignore | 1 + .../src/transforms/server_actions.rs | 841 +++++++++--------- .../server-actions/client-graph/1/output.js | 4 +- .../server-actions/client-graph/2/output.js | 4 +- .../client-graph/2/output.stderr | 11 +- .../server-graph/10/output.stderr | 6 + .../server-actions/server-graph/18/input.js | 3 + .../server-actions/server-graph/18/output.js | 3 + .../server-graph/18/output.stderr | 8 + .../server-actions/server-graph/19/input.js | 4 + .../server-actions/server-graph/19/output.js | 4 + .../server-graph/19/output.stderr | 8 + .../server-actions/server-graph/20/input.js | 4 + .../server-actions/server-graph/20/output.js | 11 + .../server-graph/20/output.stderr | 8 + .../server-actions/server-graph/21/input.js | 4 + .../server-actions/server-graph/21/output.js | 10 + .../server-graph/21/output.stderr | 7 + 18 files changed, 500 insertions(+), 441 deletions(-) create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.stderr create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.stderr create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.stderr create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.stderr diff --git a/.prettierignore b/.prettierignore index 26506c07868d1..9d358f36aa799 100644 --- a/.prettierignore +++ b/.prettierignore @@ -23,6 +23,7 @@ crates/next-custom-transforms/tests/errors/react-server-components/client-graph/ crates/next-custom-transforms/tests/errors/react-server-components/server-graph/fake-client-entry/input.js crates/next-custom-transforms/tests/errors/server-actions/server-graph/8/input.js crates/next-custom-transforms/tests/errors/server-actions/server-graph/9/input.js +crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/input.js crates/next-custom-transforms/tests/fixture/optimize-barrel/normal/4/input.js crates/next-custom-transforms/tests/fixture/react-server-components/client-graph/client-entry/input.js crates/next-custom-transforms/tests/fixture/react-server-components/server-graph/client-entry/input.js diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 33cb9e81bc653..454707c957d39 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -34,11 +34,19 @@ pub struct Config { pub cache_kinds: FxHashSet, } +#[derive(Clone, Debug)] +enum Directive { + UseServer, + UseCache { cache_kind: RcStr }, +} + +#[derive(Clone, Debug)] enum DirectiveLocation { Module, FunctionBody, } +#[derive(Clone, Debug)] enum ServerActionsErrorKind { ExportedSyncFunction { span: Span, @@ -46,7 +54,7 @@ enum ServerActionsErrorKind { }, InlineSyncFunction { span: Span, - is_action_fn: bool, + directive: Directive, }, InlineUseCacheInClientComponent { span: Span, @@ -62,12 +70,17 @@ enum ServerActionsErrorKind { MisplacedWrappedDirective { span: Span, directive: String, + location: DirectiveLocation, }, MisspelledDirective { span: Span, directive: String, expected_directive: String, }, + MultipleDirectives { + span: Span, + location: DirectiveLocation, + }, UnknownCacheKind { span: Span, cache_kind: RcStr, @@ -86,12 +99,6 @@ enum ServerActionsErrorKind { // Using BTreeMap to ensure the order of the actions is deterministic. pub type ActionsMap = BTreeMap; -// Directive-level information about a function body -struct BodyInfo { - is_action_fn: bool, - cache_kind: Option, -} - #[tracing::instrument(level = tracing::Level::TRACE, skip_all)] pub fn server_actions(file_name: &FileName, config: Config, comments: C) -> impl Pass { visit_mut_pass(ServerActions { @@ -99,8 +106,7 @@ pub fn server_actions(file_name: &FileName, config: Config, comment comments, file_name: file_name.to_string(), start_pos: BytePos(0), - in_action_file: false, - file_cache_kind: None, + file_directive: None, in_exported_expr: false, in_default_export_decl: false, fn_decl_ident: None, @@ -150,8 +156,7 @@ struct ServerActions { comments: C, start_pos: BytePos, - in_action_file: bool, - file_cache_kind: Option, + file_directive: Option, in_exported_expr: bool, in_default_export_decl: bool, fn_decl_ident: Option, @@ -310,55 +315,63 @@ impl ServerActions { }) } - // Check if the function or arrow function is an action or cache function - fn get_body_info(&mut self, maybe_body: Option<&mut BlockStmt>) -> BodyInfo { - let mut is_action_fn = false; - let mut cache_kind = None; + // Check if the function or arrow function is an action or cache function, + // and remove any server function directive. + fn get_directive_for_function( + &mut self, + maybe_body: Option<&mut BlockStmt>, + ) -> Option { + let mut directive: Option = None; // Even if it's a file-level action or cache module, the function body // might still have directives that override the module-level annotations. - - // Check if the function has a directive. if let Some(body) = maybe_body { - let mut span = None; - remove_server_directive_index_in_fn( - &mut body.stmts, - &mut is_action_fn, - &mut cache_kind, - &mut span, - &self.config, - ); + let directive_visitor = &mut DirectiveVisitor { + config: &self.config, + directive: None, + has_file_directive: self.file_directive.is_some(), + is_allowed_position: true, + location: DirectiveLocation::FunctionBody, + }; - if !self.config.is_react_server_layer { - if is_action_fn && !self.in_action_file { - emit_error(ServerActionsErrorKind::InlineUseServerInClientComponent { - span: span.unwrap_or(body.span), - }) - } + body.stmts.retain(|stmt| { + let has_directive = directive_visitor.visit_stmt(stmt); - if cache_kind.is_some() && self.file_cache_kind.is_none() && !self.in_action_file { - emit_error(ServerActionsErrorKind::InlineUseCacheInClientComponent { - span: span.unwrap_or(body.span), - }); - } - } - } + !has_directive + }); - // Self-annotations take precedence over module-level annotations. - if self.in_exported_expr && !is_action_fn && cache_kind.is_none() { - if self.in_action_file { - // All export functions in a server file are actions - is_action_fn = true; - } else if let Some(cache_file_type) = &self.file_cache_kind { - // All export functions in a cache file are cache functions - cache_kind = Some(cache_file_type.clone()); - } + directive = directive_visitor.directive.clone(); } - BodyInfo { - is_action_fn, - cache_kind, + // All exported functions inherit the file directive if they don't have their own directive. + if self.in_exported_expr && directive.is_none() && self.file_directive.is_some() { + return self.file_directive.clone(); } + + directive + } + + fn get_directive_for_module(&mut self, stmts: &mut Vec) -> Option { + let directive_visitor = &mut DirectiveVisitor { + config: &self.config, + directive: None, + has_file_directive: false, + is_allowed_position: true, + location: DirectiveLocation::Module, + }; + + stmts.retain(|item| { + if let ModuleItem::Stmt(stmt) = item { + let has_directive = directive_visitor.visit_stmt(stmt); + + !has_directive + } else { + directive_visitor.is_allowed_position = false; + true + } + }); + + directive_visitor.directive.clone() } fn maybe_hoist_and_create_proxy_for_server_action_arrow_expr( @@ -625,7 +638,7 @@ impl ServerActions { fn maybe_hoist_and_create_proxy_for_cache_arrow_expr( &mut self, ids_from_closure: Vec, - cache_kind: &str, + cache_kind: RcStr, arrow: &mut ArrowExpr, ) -> Box { let mut new_params: Vec = vec![]; @@ -652,7 +665,6 @@ impl ServerActions { let reference_id = self.generate_server_reference_id(&export_name, true, Some(&new_params)); self.has_cache = true; - self.has_action = true; self.export_actions .push((export_name.to_string(), reference_id.clone())); @@ -697,7 +709,7 @@ impl ServerActions { ..Default::default() }), })), - cache_kind, + &cache_kind, &reference_id, ids_from_closure.len(), )), @@ -761,7 +773,7 @@ impl ServerActions { &mut self, ids_from_closure: Vec, fn_name: Option, - cache_kind: &str, + cache_kind: RcStr, function: &mut Function, ) -> Box { let mut new_params: Vec = vec![]; @@ -787,7 +799,6 @@ impl ServerActions { let reference_id = self.generate_server_reference_id(&cache_name, true, Some(&new_params)); self.has_cache = true; - self.has_action = true; self.export_actions .push((cache_name.to_string(), reference_id.clone())); @@ -821,7 +832,7 @@ impl ServerActions { ..function.take() }), })), - cache_kind, + &cache_kind, &reference_id, ids_from_closure.len(), )), @@ -916,11 +927,7 @@ impl VisitMut for ServerActions { } fn visit_mut_function(&mut self, f: &mut Function) { - let BodyInfo { - is_action_fn, - cache_kind, - } = self.get_body_info(f.body.as_mut()); - + let directive = self.get_directive_for_function(f.body.as_mut()); let declared_idents_until = self.declared_idents.len(); let current_names = take(&mut self.names); @@ -932,8 +939,7 @@ impl VisitMut for ServerActions { let old_in_default_export_decl = self.in_default_export_decl; let old_fn_decl_ident = self.fn_decl_ident.clone(); self.in_module_level = false; - self.should_track_names = - is_action_fn || cache_kind.is_some() || self.should_track_names; + self.should_track_names = directive.is_some() || self.should_track_names; self.in_exported_expr = false; self.in_default_export_decl = false; self.fn_decl_ident = None; @@ -945,6 +951,10 @@ impl VisitMut for ServerActions { self.fn_decl_ident = old_fn_decl_ident; } + if !self.config.is_react_server_layer { + return; + } + let mut child_names = if self.should_track_names { let names = take(&mut self.names); self.names = current_names; @@ -954,98 +964,96 @@ impl VisitMut for ServerActions { take(&mut self.names) }; - if (is_action_fn || cache_kind.is_some()) && !f.is_async { - emit_error(ServerActionsErrorKind::InlineSyncFunction { - span: f.span, - is_action_fn, - }); - - return; - } + if let Some(directive) = directive { + if !f.is_async { + emit_error(ServerActionsErrorKind::InlineSyncFunction { + span: f.span, + directive, + }); - if !is_action_fn && cache_kind.is_none() || !self.config.is_react_server_layer { - return; - } + return; + } - if let Some(cache_kind_str) = cache_kind { - // Collect all the identifiers defined inside the closure and used - // in the cache function. With deduplication. - retain_names_from_declared_idents( - &mut child_names, - &self.declared_idents[..declared_idents_until], - ); + if let Directive::UseCache { cache_kind } = directive { + // Collect all the identifiers defined inside the closure and used + // in the cache function. With deduplication. + retain_names_from_declared_idents( + &mut child_names, + &self.declared_idents[..declared_idents_until], + ); - let new_expr = self.maybe_hoist_and_create_proxy_for_cache_function( - child_names.clone(), - self.fn_decl_ident - .clone() - .or(self.arrow_or_fn_expr_ident.clone()), - cache_kind_str.as_str(), - f, - ); + let new_expr = self.maybe_hoist_and_create_proxy_for_cache_function( + child_names.clone(), + self.fn_decl_ident + .clone() + .or(self.arrow_or_fn_expr_ident.clone()), + cache_kind, + f, + ); - if self.in_default_export_decl { - // This function expression is also the default export: - // `export default async function() {}` - // This specific case (default export) isn't handled by `visit_mut_expr`. - // Replace the original function expr with a action proxy expr. - self.rewrite_default_fn_expr_to_proxy_expr = Some(new_expr); - } else if let Some(ident) = &self.fn_decl_ident { - // Replace the original function declaration with a cache decl. - self.rewrite_fn_decl_to_proxy_decl = Some(VarDecl { - span: DUMMY_SP, - kind: VarDeclKind::Var, - decls: vec![VarDeclarator { + if self.in_default_export_decl { + // This function expression is also the default export: + // `export default async function() {}` + // This specific case (default export) isn't handled by `visit_mut_expr`. + // Replace the original function expr with a action proxy expr. + self.rewrite_default_fn_expr_to_proxy_expr = Some(new_expr); + } else if let Some(ident) = &self.fn_decl_ident { + // Replace the original function declaration with a cache decl. + self.rewrite_fn_decl_to_proxy_decl = Some(VarDecl { span: DUMMY_SP, - name: Pat::Ident(ident.clone().into()), - init: Some(new_expr), - definite: false, - }], - ..Default::default() - }); - } else { - self.rewrite_expr_to_proxy_expr = Some(new_expr); - } - } - - if is_action_fn && !(self.in_action_file && self.in_exported_expr) { - // Collect all the identifiers defined inside the closure and used - // in the action function. With deduplication. - retain_names_from_declared_idents( - &mut child_names, - &self.declared_idents[..declared_idents_until], - ); + kind: VarDeclKind::Var, + decls: vec![VarDeclarator { + span: DUMMY_SP, + name: Pat::Ident(ident.clone().into()), + init: Some(new_expr), + definite: false, + }], + ..Default::default() + }); + } else { + self.rewrite_expr_to_proxy_expr = Some(new_expr); + } + } else if !(matches!(self.file_directive, Some(Directive::UseServer)) + && self.in_exported_expr) + { + // Collect all the identifiers defined inside the closure and used + // in the action function. With deduplication. + retain_names_from_declared_idents( + &mut child_names, + &self.declared_idents[..declared_idents_until], + ); - let new_expr = self.maybe_hoist_and_create_proxy_for_server_action_function( - child_names, - f, - self.fn_decl_ident - .clone() - .or(self.arrow_or_fn_expr_ident.clone()), - ); + let new_expr = self.maybe_hoist_and_create_proxy_for_server_action_function( + child_names, + f, + self.fn_decl_ident + .clone() + .or(self.arrow_or_fn_expr_ident.clone()), + ); - if self.in_default_export_decl { - // This function expression is also the default export: - // `export default async function() {}` - // This specific case (default export) isn't handled by `visit_mut_expr`. - // Replace the original function expr with a action proxy expr. - self.rewrite_default_fn_expr_to_proxy_expr = Some(new_expr); - } else if let Some(ident) = &self.fn_decl_ident { - // Replace the original function declaration with an action proxy declaration - // expr. - self.rewrite_fn_decl_to_proxy_decl = Some(VarDecl { - span: DUMMY_SP, - kind: VarDeclKind::Var, - decls: vec![VarDeclarator { + if self.in_default_export_decl { + // This function expression is also the default export: + // `export default async function() {}` + // This specific case (default export) isn't handled by `visit_mut_expr`. + // Replace the original function expr with a action proxy expr. + self.rewrite_default_fn_expr_to_proxy_expr = Some(new_expr); + } else if let Some(ident) = &self.fn_decl_ident { + // Replace the original function declaration with an action proxy declaration + // expr. + self.rewrite_fn_decl_to_proxy_decl = Some(VarDecl { span: DUMMY_SP, - name: Pat::Ident(ident.clone().into()), - init: Some(new_expr), - definite: false, - }], - ..Default::default() - }); - } else { - self.rewrite_expr_to_proxy_expr = Some(new_expr); + kind: VarDeclKind::Var, + decls: vec![VarDeclarator { + span: DUMMY_SP, + name: Pat::Ident(ident.clone().into()), + init: Some(new_expr), + definite: false, + }], + ..Default::default() + }); + } else { + self.rewrite_expr_to_proxy_expr = Some(new_expr); + } } } } @@ -1075,14 +1083,13 @@ impl VisitMut for ServerActions { fn visit_mut_arrow_expr(&mut self, a: &mut ArrowExpr) { // Arrow expressions need to be visited in prepass to determine if it's // an action function or not. - let BodyInfo { - is_action_fn, - cache_kind, - } = self.get_body_info(if let BlockStmtOrExpr::BlockStmt(block) = &mut *a.body { - Some(block) - } else { - None - }); + let directive = self.get_directive_for_function( + if let BlockStmtOrExpr::BlockStmt(block) = &mut *a.body { + Some(block) + } else { + None + }, + ); let declared_idents_until = self.declared_idents.len(); let current_names = take(&mut self.names); @@ -1094,8 +1101,7 @@ impl VisitMut for ServerActions { let old_in_exported_expr = self.in_exported_expr; let old_in_default_export_decl = self.in_default_export_decl; self.in_module_level = false; - self.should_track_names = - is_action_fn || cache_kind.is_some() || self.should_track_names; + self.should_track_names = directive.is_some() || self.should_track_names; self.in_exported_expr = false; self.in_default_export_decl = false; { @@ -1110,6 +1116,10 @@ impl VisitMut for ServerActions { self.in_default_export_decl = old_in_default_export_decl; } + if !self.config.is_react_server_layer { + return; + } + let mut child_names = if self.should_track_names { let names = take(&mut self.names); self.names = current_names; @@ -1119,39 +1129,36 @@ impl VisitMut for ServerActions { take(&mut self.names) }; - if !a.is_async && (is_action_fn || cache_kind.is_some()) { - emit_error(ServerActionsErrorKind::InlineSyncFunction { - span: a.span, - is_action_fn, - }); - - return; - } - - if !is_action_fn && cache_kind.is_none() || !self.config.is_react_server_layer { - return; - } + if let Some(directive) = directive { + if !a.is_async { + emit_error(ServerActionsErrorKind::InlineSyncFunction { + span: a.span, + directive, + }); - // Collect all the identifiers defined inside the closure and used - // in the action function. With deduplication. - retain_names_from_declared_idents( - &mut child_names, - &self.declared_idents[..declared_idents_until], - ); + return; + } - let maybe_new_expr = if is_action_fn && !self.in_action_file { - Some(self.maybe_hoist_and_create_proxy_for_server_action_arrow_expr(child_names, a)) - } else { - cache_kind.map(|cache_kind_str| { - self.maybe_hoist_and_create_proxy_for_cache_arrow_expr( - child_names, - cache_kind_str.as_str(), - a, - ) - }) - }; + // Collect all the identifiers defined inside the closure and used + // in the action function. With deduplication. + retain_names_from_declared_idents( + &mut child_names, + &self.declared_idents[..declared_idents_until], + ); - self.rewrite_expr_to_proxy_expr = maybe_new_expr; + if let Directive::UseCache { cache_kind } = directive { + self.rewrite_expr_to_proxy_expr = + Some(self.maybe_hoist_and_create_proxy_for_cache_arrow_expr( + child_names, + cache_kind, + a, + )); + } else if !matches!(self.file_directive, Some(Directive::UseServer)) { + self.rewrite_expr_to_proxy_expr = Some( + self.maybe_hoist_and_create_proxy_for_server_action_arrow_expr(child_names, a), + ); + } + } } fn visit_mut_module(&mut self, m: &mut Module) { @@ -1260,26 +1267,25 @@ impl VisitMut for ServerActions { } fn visit_mut_module_items(&mut self, stmts: &mut Vec) { - remove_server_directive_index_in_module( - stmts, - &mut self.in_action_file, - &mut self.file_cache_kind, - &mut self.has_action, - &mut self.has_cache, - &self.config, - ); - - // If we're in a "use cache" file, collect all original IDs from export - // specifiers in a pre-pass so that we know which functions are - // exported, e.g. for this case: - // ``` - // "use cache" - // function foo() {} - // function Bar() {} - // export { foo } - // export default Bar - // ``` - if self.file_cache_kind.is_some() { + self.file_directive = self.get_directive_for_module(stmts); + + let in_cache_file = matches!(self.file_directive, Some(Directive::UseCache { .. })); + let in_action_file = matches!(self.file_directive, Some(Directive::UseServer)); + + self.has_action = in_action_file; + self.has_cache = in_cache_file; + + if in_cache_file { + // If we're in a "use cache" file, collect all original IDs from + // export specifiers in a pre-pass so that we know which functions + // are exported, e.g. for this case: + // ``` + // "use cache" + // function foo() {} + // function Bar() {} + // export { foo } + // export default Bar + // ``` for stmt in stmts.iter() { match stmt { ModuleItem::ModuleDecl(ModuleDecl::ExportDefaultExpr(export_default_expr)) => { @@ -1306,8 +1312,7 @@ impl VisitMut for ServerActions { } // Only track exported identifiers in action files or cache files. - let is_cache_file = self.file_cache_kind.is_some(); - let should_track_exports = self.in_action_file || is_cache_file; + let should_track_exports = self.file_directive.is_some(); let old_annotations = self.annotations.take(); let mut new = Vec::with_capacity(stmts.len()); @@ -1333,7 +1338,7 @@ impl VisitMut for ServerActions { } else if is_cache_fn { true } else { - is_cache_file + in_cache_file }; // If it's a self-annotated cache function, we need to skip @@ -1364,7 +1369,7 @@ impl VisitMut for ServerActions { ident.sym.to_string(), self.generate_server_reference_id( ident.sym.as_ref(), - is_cache_file, + in_cache_file, None, ), )); @@ -1405,7 +1410,7 @@ impl VisitMut for ServerActions { sym.to_string(), self.generate_server_reference_id( sym.as_ref(), - is_cache_file, + in_cache_file, None, ), )); @@ -1416,7 +1421,7 @@ impl VisitMut for ServerActions { str.value.to_string(), self.generate_server_reference_id( str.value.as_ref(), - is_cache_file, + in_cache_file, None, ), )); @@ -1428,7 +1433,7 @@ impl VisitMut for ServerActions { ident.sym.to_string(), self.generate_server_reference_id( ident.sym.as_ref(), - is_cache_file, + in_cache_file, None, ), )); @@ -1452,7 +1457,7 @@ impl VisitMut for ServerActions { } else if is_cache_fn { true } else { - is_cache_file + in_cache_file }; // If it's a self-annotated cache function, we need to skip @@ -1527,7 +1532,7 @@ impl VisitMut for ServerActions { } else if is_cache_fn { true } else { - is_cache_file + in_cache_file }; // If it's a self-annotated cache function, we need to skip @@ -1576,7 +1581,7 @@ impl VisitMut for ServerActions { "default".into(), self.generate_server_reference_id( "default", - is_cache_file, + in_cache_file, None, ), )); @@ -1594,7 +1599,7 @@ impl VisitMut for ServerActions { "default".into(), self.generate_server_reference_id( "default", - is_cache_file, + in_cache_file, None, ), )); @@ -1619,7 +1624,7 @@ impl VisitMut for ServerActions { if disallowed_export_span != DUMMY_SP { emit_error(ServerActionsErrorKind::ExportedSyncFunction { span: disallowed_export_span, - in_action_file: self.in_action_file, + in_action_file, }); return; @@ -1640,9 +1645,7 @@ impl VisitMut for ServerActions { self.rewrite_default_fn_expr_to_proxy_expr = None; } - if self.config.is_react_server_layer - || (!self.in_action_file && self.file_cache_kind.is_none()) - { + if self.config.is_react_server_layer || self.file_directive.is_none() { new.append(&mut self.hoisted_extra_items); new.push(new_stmt); new.extend(self.annotations.drain(..).map(ModuleItem::Stmt)); @@ -1761,7 +1764,7 @@ impl VisitMut for ServerActions { })); new.push(export_expr); } - } else if self.file_cache_kind.is_none() { + } else if !in_cache_file { self.annotations.push(Stmt::Expr(ExprStmt { span: DUMMY_SP, expr: Box::new(annotate_ident_as_server_reference( @@ -1785,7 +1788,7 @@ impl VisitMut for ServerActions { new.append(&mut self.extra_items); // For "use cache" files, there's no need to do extra annotations. - if self.file_cache_kind.is_none() && !self.exported_idents.is_empty() { + if !in_cache_file && !self.exported_idents.is_empty() { let ensure_ident = private_ident!("ensureServerEntryExports"); new.push(ModuleItem::ModuleDecl(ModuleDecl::Import(ImportDecl { span: DUMMY_SP, @@ -1839,9 +1842,7 @@ impl VisitMut for ServerActions { let mut actions = self.export_actions.clone(); // All exported values are considered as actions if the file is an action file. - if self.in_action_file - || self.file_cache_kind.is_some() && !self.config.is_react_server_layer - { + if in_action_file || in_cache_file && !self.config.is_react_server_layer { actions.extend( self.exported_idents .iter() @@ -2313,125 +2314,6 @@ fn detect_similar_strings(a: &str, b: &str) -> bool { } } -fn remove_server_directive_index_in_module( - stmts: &mut Vec, - in_action_file: &mut bool, - file_cache_kind: &mut Option, - has_action: &mut bool, - has_cache: &mut bool, - config: &Config, -) { - let mut is_directive = true; - - stmts.retain(|stmt| { - match stmt { - ModuleItem::Stmt(Stmt::Expr(ExprStmt { - expr: box Expr::Lit(Lit::Str(Str { value, span, .. })), - .. - })) => { - if value == "use server" { - if is_directive { - *in_action_file = true; - *has_action = true; - return false; - } else { - emit_error(ServerActionsErrorKind::MisplacedDirective { - span: *span, - directive: value.to_string(), - location: DirectiveLocation::Module, - }); - } - } else - // `use cache` or `use cache: foo` - if value == "use cache" || value.starts_with("use cache: ") { - if is_directive { - if !config.dynamic_io_enabled { - emit_error(ServerActionsErrorKind::UseCacheWithoutDynamicIO { - span: *span, - directive: value.to_string(), - }); - } - - if value == "use cache" { - *file_cache_kind = Some("default".into()); - } else { - // Slice the value after "use cache: " - let cache_kind_str = RcStr::from(value.split_at("use cache: ".len()).1); - - if !config.cache_kinds.contains(&cache_kind_str) { - emit_error(ServerActionsErrorKind::UnknownCacheKind { - span: *span, - cache_kind: cache_kind_str.clone(), - }); - } - - *file_cache_kind = Some(cache_kind_str) - } - - *has_cache = true; - return false; - } else { - emit_error(ServerActionsErrorKind::MisplacedDirective { - span: *span, - directive: value.to_string(), - location: DirectiveLocation::Module, - }); - } - } else { - // Detect typo of "use cache" - if detect_similar_strings(value, "use cache") { - emit_error(ServerActionsErrorKind::MisspelledDirective { - span: *span, - directive: value.to_string(), - expected_directive: "use cache".to_string(), - }); - } - } - } - ModuleItem::Stmt(Stmt::Expr(ExprStmt { - expr: - box Expr::Paren(ParenExpr { - expr: box Expr::Lit(Lit::Str(Str { value, .. })), - .. - }), - span, - .. - })) => { - // Match `("use server")`. - if value == "use server" || detect_similar_strings(value, "use server") { - if is_directive { - emit_error(ServerActionsErrorKind::WrappedDirective { - span: *span, - directive: "use server".to_string(), - }); - } else { - emit_error(ServerActionsErrorKind::MisplacedWrappedDirective { - span: *span, - directive: "use server".to_string(), - }); - } - } else if value == "use cache" || detect_similar_strings(value, "use cache") { - if is_directive { - emit_error(ServerActionsErrorKind::WrappedDirective { - span: *span, - directive: "use cache".to_string(), - }); - } else { - emit_error(ServerActionsErrorKind::MisplacedWrappedDirective { - span: *span, - directive: "use cache".to_string(), - }); - } - } - } - _ => { - is_directive = false; - } - } - true - }); -} - // Check if the function or arrow function has any action or cache directives, // without mutating the function body or erroring out. // This is used to quickly determine if we need to use the module-level @@ -2463,88 +2345,6 @@ fn has_body_directive(maybe_body: &Option) -> (bool, bool) { (is_action_fn, is_cache_fn) } -fn remove_server_directive_index_in_fn( - stmts: &mut Vec, - is_action_fn: &mut bool, - cache_kind: &mut Option, - action_span: &mut Option, - config: &Config, -) { - let mut is_directive = true; - - stmts.retain(|stmt| { - if let Stmt::Expr(ExprStmt { - expr: box Expr::Lit(Lit::Str(Str { value, span, .. })), - .. - }) = stmt - { - if value == "use server" { - *action_span = Some(*span); - - if is_directive { - *is_action_fn = true; - return false; - } else { - emit_error(ServerActionsErrorKind::MisplacedDirective { - span: *span, - directive: value.to_string(), - location: DirectiveLocation::FunctionBody, - }); - } - } else if detect_similar_strings(value, "use server") { - // Detect typo of "use server" - emit_error(ServerActionsErrorKind::MisspelledDirective { - span: *span, - directive: value.to_string(), - expected_directive: "use server".to_string(), - }); - } else if value == "use cache" || value.starts_with("use cache: ") { - if is_directive { - if !config.dynamic_io_enabled { - emit_error(ServerActionsErrorKind::UseCacheWithoutDynamicIO { - span: *span, - directive: value.to_string(), - }); - } - - if value == "use cache" { - *cache_kind = Some("default".into()); - } else { - // Slice the value after "use cache: " - let cache_kind_str = RcStr::from(value.split_at("use cache: ".len()).1); - - if !config.cache_kinds.contains(&cache_kind_str) { - emit_error(ServerActionsErrorKind::UnknownCacheKind { - span: *span, - cache_kind: cache_kind_str.clone(), - }); - } - - *cache_kind = Some(cache_kind_str); - }; - return false; - } else { - emit_error(ServerActionsErrorKind::MisplacedDirective { - span: *span, - directive: value.to_string(), - location: DirectiveLocation::FunctionBody, - }); - } - } else if detect_similar_strings(value, "use cache") { - // Detect typo of "use cache" - emit_error(ServerActionsErrorKind::MisspelledDirective { - span: *span, - directive: value.to_string(), - expected_directive: "use cache".to_string(), - }); - } - } else { - is_directive = false; - } - true - }); -} - fn collect_idents_in_array_pat(elems: &[Option], idents: &mut Vec) { for elem in elems.iter().flatten() { match elem { @@ -2642,6 +2442,161 @@ fn collect_decl_idents_in_stmt(stmt: &Stmt, idents: &mut Vec) { } } +struct DirectiveVisitor<'a> { + config: &'a Config, + location: DirectiveLocation, + directive: Option, + has_file_directive: bool, + is_allowed_position: bool, +} + +impl DirectiveVisitor<'_> { + /** + * Returns `true` if the statement contains a server directive. + * The found directive is assigned to `DirectiveVisitor::directive`. + */ + fn visit_stmt(&mut self, stmt: &Stmt) -> bool { + let in_fn_body = matches!(self.location, DirectiveLocation::FunctionBody); + let allow_inline = self.config.is_react_server_layer || self.has_file_directive; + + match stmt { + Stmt::Expr(ExprStmt { + expr: box Expr::Lit(Lit::Str(Str { value, span, .. })), + .. + }) => { + if value == "use server" { + if in_fn_body && !allow_inline { + emit_error(ServerActionsErrorKind::InlineUseServerInClientComponent { + span: *span, + }) + } else if let Some(Directive::UseCache { .. }) = self.directive { + emit_error(ServerActionsErrorKind::MultipleDirectives { + span: *span, + location: self.location.clone(), + }); + } else if self.is_allowed_position { + self.directive = Some(Directive::UseServer); + + return true; + } else { + emit_error(ServerActionsErrorKind::MisplacedDirective { + span: *span, + directive: value.to_string(), + location: self.location.clone(), + }); + } + } else if detect_similar_strings(value, "use server") { + // Detect typo of "use server" + emit_error(ServerActionsErrorKind::MisspelledDirective { + span: *span, + directive: value.to_string(), + expected_directive: "use server".to_string(), + }); + } else + // `use cache` or `use cache: foo` + if value == "use cache" || value.starts_with("use cache: ") { + if in_fn_body && !allow_inline { + emit_error(ServerActionsErrorKind::InlineUseCacheInClientComponent { + span: *span, + }) + } else if let Some(Directive::UseServer) = self.directive { + emit_error(ServerActionsErrorKind::MultipleDirectives { + span: *span, + location: self.location.clone(), + }); + } else if self.is_allowed_position { + if !self.config.dynamic_io_enabled { + emit_error(ServerActionsErrorKind::UseCacheWithoutDynamicIO { + span: *span, + directive: value.to_string(), + }); + } + + if value == "use cache" { + self.directive = Some(Directive::UseCache { + cache_kind: RcStr::from("default"), + }); + } else { + // Slice the value after "use cache: " + let cache_kind = RcStr::from(value.split_at("use cache: ".len()).1); + + if !self.config.cache_kinds.contains(&cache_kind) { + emit_error(ServerActionsErrorKind::UnknownCacheKind { + span: *span, + cache_kind: cache_kind.clone(), + }); + } + + self.directive = Some(Directive::UseCache { cache_kind }); + } + + return true; + } else { + emit_error(ServerActionsErrorKind::MisplacedDirective { + span: *span, + directive: value.to_string(), + location: self.location.clone(), + }); + } + } else { + // Detect typo of "use cache" + if detect_similar_strings(value, "use cache") { + emit_error(ServerActionsErrorKind::MisspelledDirective { + span: *span, + directive: value.to_string(), + expected_directive: "use cache".to_string(), + }); + } + } + } + Stmt::Expr(ExprStmt { + expr: + box Expr::Paren(ParenExpr { + expr: box Expr::Lit(Lit::Str(Str { value, .. })), + .. + }), + span, + .. + }) => { + // Match `("use server")`. + if value == "use server" || detect_similar_strings(value, "use server") { + if self.is_allowed_position { + emit_error(ServerActionsErrorKind::WrappedDirective { + span: *span, + directive: "use server".to_string(), + }); + } else { + emit_error(ServerActionsErrorKind::MisplacedWrappedDirective { + span: *span, + directive: "use server".to_string(), + location: self.location.clone(), + }); + } + } else if value == "use cache" || detect_similar_strings(value, "use cache") { + if self.is_allowed_position { + emit_error(ServerActionsErrorKind::WrappedDirective { + span: *span, + directive: "use cache".to_string(), + }); + } else { + emit_error(ServerActionsErrorKind::MisplacedWrappedDirective { + span: *span, + directive: "use cache".to_string(), + location: self.location.clone(), + }); + } + } + } + _ => { + // Directives must not be placed after other statements. + self.is_allowed_position = false; + } + }; + + false + } +} + pub(crate) struct ClosureReplacer<'a> { used_ids: &'a [Name], private_ctxt: SyntaxContext, @@ -2831,13 +2786,13 @@ fn emit_error(error_kind: ServerActionsErrorKind) { "# }, ), - ServerActionsErrorKind::InlineSyncFunction { span, is_action_fn } => ( + ServerActionsErrorKind::InlineSyncFunction { span, directive } => ( span, formatdoc! { r#" {subject} must be async functions. "#, - subject = if is_action_fn { + subject = if let Directive::UseServer = directive { "Server Actions" } else { "\"use cache\" functions" @@ -2860,12 +2815,20 @@ fn emit_error(error_kind: ServerActionsErrorKind) { } }, ), - ServerActionsErrorKind::MisplacedWrappedDirective { span, directive } => ( + ServerActionsErrorKind::MisplacedWrappedDirective { + span, + directive, + location, + } => ( span, formatdoc! { r#" - The "{directive}" directive must be at the top of the file, and cannot be wrapped in parentheses. - "# + The "{directive}" directive must be at the top of the {location}, and cannot be wrapped in parentheses. + "#, + location = match location { + DirectiveLocation::Module => "file", + DirectiveLocation::FunctionBody => "function body", + } }, ), ServerActionsErrorKind::MisspelledDirective { @@ -2880,6 +2843,18 @@ fn emit_error(error_kind: ServerActionsErrorKind) { "# }, ), + ServerActionsErrorKind::MultipleDirectives { span, location } => ( + span, + formatdoc! { + r#" + Conflicting directives "use server" and "use cache" found in the same {location}. You cannot place both directives at the top of a {location}. Please remove one of them. + "#, + location = match location { + DirectiveLocation::Module => "file", + DirectiveLocation::FunctionBody => "function body", + } + }, + ), ServerActionsErrorKind::UnknownCacheKind { span, cache_kind } => ( span, formatdoc! { diff --git a/crates/next-custom-transforms/tests/errors/server-actions/client-graph/1/output.js b/crates/next-custom-transforms/tests/errors/server-actions/client-graph/1/output.js index 175462f2e0522..a05fa05acd5c4 100644 --- a/crates/next-custom-transforms/tests/errors/server-actions/client-graph/1/output.js +++ b/crates/next-custom-transforms/tests/errors/server-actions/client-graph/1/output.js @@ -1,4 +1,6 @@ /* __next_internal_client_entry_do_not_use__ default auto */ export default function App() { - async function fn() {} + async function fn() { + 'use server'; + } return
App
; } diff --git a/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.js b/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.js index 175462f2e0522..dcbfb41c1fe3a 100644 --- a/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.js +++ b/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.js @@ -1,4 +1,6 @@ /* __next_internal_client_entry_do_not_use__ default auto */ export default function App() { - async function fn() {} + async function fn() { + 'use cache'; + } return
App
; } diff --git a/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.stderr index 124a54458f946..df9128956308b 100644 --- a/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.stderr +++ b/crates/next-custom-transforms/tests/errors/server-actions/client-graph/2/output.stderr @@ -2,10 +2,9 @@ | To use "use cache" functions in a Client Component, you can either export them from a separate file with "use cache" or "use server" at the top, or pass them down through props from a Server | Component. | - ,-[input.js:4:1] - 3 | export default function App() { - 4 | ,-> async function fn() { - 5 | | 'use cache' - 6 | `-> } - 7 | return
App
+ ,-[input.js:5:1] + 4 | async function fn() { + 5 | 'use cache' + : ^^^^^^^^^^^ + 6 | } `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/10/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/10/output.stderr index e69de29bb2d1d..8cf44ceba058e 100644 --- a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/10/output.stderr +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/10/output.stderr @@ -0,0 +1,6 @@ + x Did you mean "use server"? "use sevrer" is not a supported directive name." + | + ,-[input.js:1:1] + 1 | 'use sevrer' + : ^^^^^^^^^^^^ + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/input.js new file mode 100644 index 0000000000000..593d767b5695d --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/input.js @@ -0,0 +1,3 @@ +export async function fn() { + ('use cache') +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.js new file mode 100644 index 0000000000000..11ae6acf1cbd4 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.js @@ -0,0 +1,3 @@ +export async function fn() { + 'use cache'; +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.stderr new file mode 100644 index 0000000000000..606cbe3cc086f --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/18/output.stderr @@ -0,0 +1,8 @@ + x The "use cache" directive cannot be wrapped in parentheses. + | + ,-[input.js:2:1] + 1 | export async function fn() { + 2 | ('use cache') + : ^^^^^^^^^^^^^ + 3 | } + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/input.js new file mode 100644 index 0000000000000..25f7980ec6a5b --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/input.js @@ -0,0 +1,4 @@ +export async function fn() { + console.log('foo') + ;('use cache') +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.js new file mode 100644 index 0000000000000..0cefd89bf8837 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.js @@ -0,0 +1,4 @@ +export async function fn() { + console.log('foo'); + 'use cache'; +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.stderr new file mode 100644 index 0000000000000..667f8b5f766d1 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/19/output.stderr @@ -0,0 +1,8 @@ + x The "use cache" directive must be at the top of the function body, and cannot be wrapped in parentheses. + | + ,-[input.js:3:1] + 2 | console.log('foo') + 3 | ;('use cache') + : ^^^^^^^^^^^^^ + 4 | } + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/input.js new file mode 100644 index 0000000000000..7653ecad0057f --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/input.js @@ -0,0 +1,4 @@ +export async function fn() { + 'use cache' + 'use server' +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.js new file mode 100644 index 0000000000000..ae58732828e81 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.js @@ -0,0 +1,11 @@ +/* __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 fn() { + 'use server'; +}); +Object.defineProperty($$RSC_SERVER_CACHE_0, "name", { + "value": "fn", + "writable": false +}); +export var fn = registerServerReference($$RSC_SERVER_CACHE_0, "803128060c414d59f8552e4788b846c0d2b7f74743", null); diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.stderr new file mode 100644 index 0000000000000..50bb184194f11 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/20/output.stderr @@ -0,0 +1,8 @@ + x Conflicting directives "use server" and "use cache" found in the same function body. You cannot place both directives at the top of a function body. Please remove one of them. + | + ,-[input.js:3:1] + 2 | 'use cache' + 3 | 'use server' + : ^^^^^^^^^^^^ + 4 | } + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/input.js new file mode 100644 index 0000000000000..0f3bfecd2abb9 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/input.js @@ -0,0 +1,4 @@ +'use cache' +'use server' + +export async function fn() {} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.js new file mode 100644 index 0000000000000..bd5baea470b52 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.js @@ -0,0 +1,10 @@ +/* __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"; +'use server'; +export var $$RSC_SERVER_CACHE_0 = $$cache__("default", "803128060c414d59f8552e4788b846c0d2b7f74743", 0, /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ async function fn() {}); +Object.defineProperty($$RSC_SERVER_CACHE_0, "name", { + "value": "fn", + "writable": false +}); +export var fn = registerServerReference($$RSC_SERVER_CACHE_0, "803128060c414d59f8552e4788b846c0d2b7f74743", null); diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.stderr new file mode 100644 index 0000000000000..3ae0a97828df5 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/21/output.stderr @@ -0,0 +1,7 @@ + x Conflicting directives "use server" and "use cache" found in the same file. You cannot place both directives at the top of a file. Please remove one of them. + | + ,-[input.js:2:1] + 1 | 'use cache' + 2 | 'use server' + : ^^^^^^^^^^^^ + `----