Skip to content

Commit

Permalink
Improve the RSC directive transform (#70562)
Browse files Browse the repository at this point in the history 
This PR extends the cache directive to accept an additional type
parameter which defaults to `"default"`.

Several follow-ups:
- Accept different syntaxes (like places of spaces) and typo detection;
- Have a limited set of valid options with better errors.
  • Loading branch information
@shuding
shuding authored Sep 28, 2024
1 parent 34e6f48 commit 5c235de
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 83 deletions.
216 changes: 134 additions & 82 deletions crates/next-custom-transforms/src/transforms/server_actions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub fn server_actions<C: Comments>(
file_name: file_name.to_string(),
start_pos: BytePos(0),
in_action_file: false,
in_cache_file: false,
in_cache_file: None,
in_export_decl: false,
in_default_export_decl: false,
in_callee: false,
Expand Down Expand Up @@ -93,7 +93,7 @@ struct ServerActions<C: Comments> {

start_pos: BytePos,
in_action_file: bool,
in_cache_file: bool,
in_cache_file: Option<String>,
in_export_decl: bool,
in_default_export_decl: bool,
in_callee: bool,
Expand Down Expand Up @@ -125,42 +125,62 @@ struct ServerActions<C: Comments> {

impl<C: Comments> ServerActions<C> {
// Check if the function or arrow function is an action function
fn get_body_info(&mut self, maybe_body: Option<&mut BlockStmt>) -> (bool, bool) {
fn get_body_info(&mut self, maybe_body: Option<&mut BlockStmt>) -> (bool, Option<String>) {
let mut is_action_fn = false;
let mut is_cache_fn = false;

if self.in_action_file && self.in_export_decl {
// All export functions in a server file are actions
is_action_fn = true;
} else if self.in_cache_file && self.in_export_decl {
// All export functions in a cache file are cache functions
is_cache_fn = true;
} else {
// Check if the function has `"use server"`
if let Some(body) = maybe_body {
let mut action_span = None;
remove_server_directive_index_in_fn(
&mut body.stmts,
&mut is_action_fn,
&mut is_cache_fn,
&mut action_span,
self.config.enabled,
);
let mut cache_type = 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_type,
&mut span,
self.config.enabled,
);

if is_action_fn && !self.config.is_react_server_layer && !self.in_action_file {
HANDLER.with(|handler| {
handler
.struct_span_err(
action_span.unwrap_or(body.span),
"It is not allowed to define inline \"use server\" annotated Server Actions in Client Components.\nTo use Server Actions in a Client Component, you can either export them from a separate file with \"use server\" at the top, or pass them down through props from a Server Component.\n\nRead more: https://nextjs.org/docs/app/api-reference/functions/server-actions#with-client-components\n",
)
.emit()
});
}
if is_action_fn && !self.config.is_react_server_layer && !self.in_action_file {
HANDLER.with(|handler| {
handler
.struct_span_err(
span.unwrap_or(body.span),
"It is not allowed to define inline \"use server\" annotated Server Actions in Client Components.\nTo use Server Actions in a Client Component, you can either export them from a separate file with \"use server\" at the top, or pass them down through props from a Server Component.\n\nRead more: https://nextjs.org/docs/app/api-reference/functions/server-actions#with-client-components\n",
)
.emit()
});
}

if cache_type.is_some()
&& !self.config.is_react_server_layer
&& self.in_cache_file.is_none()
{
HANDLER.with(|handler| {
handler
.struct_span_err(
span.unwrap_or(body.span),
"It is not allowed to define inline \"use cache\" annotated Cache \
Functions in Client Components.",
)
.emit()
});
}
}

if self.in_export_decl {
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.in_cache_file {
// All export functions in a cache file are cache functions
cache_type = Some(cache_file_type.clone());
}
}

(is_action_fn, is_cache_fn)
(is_action_fn, cache_type)
}

fn maybe_hoist_and_create_proxy(
Expand Down Expand Up @@ -437,6 +457,7 @@ impl<C: Comments> ServerActions<C> {
&mut self,
ids_from_closure: Vec<Name>,
fn_name: Option<Ident>,
cache_type: &str,
function: Option<&mut Box<Function>>,
arrow: Option<&mut ArrowExpr>,
) -> Option<Box<Expr>> {
Expand Down Expand Up @@ -588,7 +609,7 @@ impl<C: Comments> ServerActions<C> {
..Default::default()
}),
})),
"default",
cache_type,
&reference_id,
)),
definite: false,
Expand Down Expand Up @@ -706,7 +727,7 @@ impl<C: Comments> ServerActions<C> {
..*f.take()
}),
})),
"default",
cache_type,
&reference_id,
)),
definite: false,
Expand Down Expand Up @@ -753,7 +774,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
}

fn visit_mut_fn_expr(&mut self, f: &mut FnExpr) {
let (is_action_fn, is_cache_fn) = self.get_body_info(f.function.body.as_mut());
let (is_action_fn, cache_type) = self.get_body_info(f.function.body.as_mut());

let declared_idents_until = self.declared_idents.len();
let current_names = take(&mut self.names);
Expand Down Expand Up @@ -784,47 +805,50 @@ impl<C: Comments> VisitMut for ServerActions<C> {
take(&mut self.names)
};

if (is_action_fn || is_cache_fn) && !f.function.is_async {
if (is_action_fn || cache_type.is_some()) && !f.function.is_async {
HANDLER.with(|handler| {
handler
.struct_span_err(f.function.span, "Server actions must be async functions")
.emit();
});
}

if is_cache_fn && !(self.in_cache_file && self.in_export_decl) {
// It's a cache function. If it doesn't have a name, give it one.
match f.ident.as_mut() {
None => {
let action_name = gen_cache_ident(&mut self.reference_index);
let ident = Ident::new(action_name, DUMMY_SP, Default::default());
f.ident.insert(ident)
}
Some(i) => i,
};
if let Some(cache_type_str) = cache_type {
if !(self.in_cache_file.is_some() && self.in_export_decl) {
// It's a cache function. If it doesn't have a name, give it one.
match f.ident.as_mut() {
None => {
let action_name = gen_cache_ident(&mut self.reference_index);
let ident = Ident::new(action_name, DUMMY_SP, Default::default());
f.ident.insert(ident)
}
Some(i) => i,
};

// 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],
);
// 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 maybe_new_expr = self.maybe_hoist_and_create_proxy_to_cache(
child_names.clone(),
f.ident.clone(),
Some(&mut f.function),
None,
);
let maybe_new_expr = self.maybe_hoist_and_create_proxy_to_cache(
child_names.clone(),
f.ident.clone(),
cache_type_str.as_str(),
Some(&mut f.function),
None,
);

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 = maybe_new_expr;
} else {
self.rewrite_expr_to_proxy_expr = maybe_new_expr;
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 = maybe_new_expr;
} else {
self.rewrite_expr_to_proxy_expr = maybe_new_expr;
}
}
}

Expand Down Expand Up @@ -873,7 +897,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
}

fn visit_mut_fn_decl(&mut self, f: &mut FnDecl) {
let (is_action_fn, is_cache_fn) = self.get_body_info(f.function.body.as_mut());
let (is_action_fn, cache_type) = self.get_body_info(f.function.body.as_mut());

let declared_idents_until = self.declared_idents.len();
let current_names = take(&mut self.names);
Expand Down Expand Up @@ -904,7 +928,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
take(&mut self.names)
};

if is_cache_fn {
if let Some(cache_type_str) = cache_type {
if !f.function.is_async {
HANDLER.with(|handler| {
handler
Expand All @@ -916,6 +940,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
let maybe_new_expr = self.maybe_hoist_and_create_proxy_to_cache(
[].to_vec(),
Some(f.ident.clone()),
cache_type_str.as_str(),
Some(&mut f.function),
None,
);
Expand Down Expand Up @@ -996,7 +1021,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
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 (is_action_fn, is_cache_fn) =
let (is_action_fn, cache_type) =
self.get_body_info(if let BlockStmtOrExpr::BlockStmt(block) = &mut *a.body {
Some(block)
} else {
Expand Down Expand Up @@ -1037,14 +1062,14 @@ impl<C: Comments> VisitMut for ServerActions<C> {
take(&mut self.names)
};

if !is_action_fn && !is_cache_fn {
if !is_action_fn && cache_type.is_none() {
return;
}

if !a.is_async && !self.in_action_file && !self.in_cache_file {
if !a.is_async && !self.in_action_file && self.in_cache_file.is_none() {
HANDLER.with(|handler| {
handler
.struct_span_err(a.span, "Server actions must be async functions")
.struct_span_err(a.span, "Server Actions must be async functions")
.emit();
});
}
Expand All @@ -1058,8 +1083,16 @@ impl<C: Comments> VisitMut for ServerActions<C> {

let maybe_new_expr = if is_action_fn {
self.maybe_hoist_and_create_proxy(child_names, None, Some(a))
} else if let Some(cache_type_str) = cache_type {
self.maybe_hoist_and_create_proxy_to_cache(
child_names,
None,
cache_type_str.as_str(),
None,
Some(a),
)
} else {
self.maybe_hoist_and_create_proxy_to_cache(child_names, None, None, Some(a))
None
};

self.rewrite_expr_to_proxy_expr = maybe_new_expr;
Expand Down Expand Up @@ -1156,7 +1189,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
for mut stmt in stmts.take() {
// For server boundary files, it's not allowed to export things other than async
// functions.
if self.in_action_file || self.in_cache_file {
if self.in_action_file || self.in_cache_file.is_some() {
let mut disallowed_export_span = DUMMY_SP;

// Currently only function exports are allowed.
Expand Down Expand Up @@ -1906,7 +1939,7 @@ fn detect_similar_strings(a: &str, b: &str) -> bool {
fn remove_server_directive_index_in_module(
stmts: &mut Vec<ModuleItem>,
in_action_file: &mut bool,
in_cache_file: &mut bool,
in_cache_file: &mut Option<String>,
has_action: &mut bool,
has_cache: &mut bool,
enabled: bool,
Expand Down Expand Up @@ -1944,9 +1977,20 @@ fn remove_server_directive_index_in_module(
.emit();
});
}
} else if value == "use cache" {
} else
// `use cache` or `use cache: foo`
if value == "use cache" || value.starts_with("use cache: ") {
if is_directive {
*in_cache_file = true;
*in_cache_file = Some(
if value == "use cache" {
"default".into()
} else {
// Slice the value after "use cache: "
value.split_at(
"use cache: ".len(),
).1.into()
}
);
*has_cache = true;
return false;
} else {
Expand Down Expand Up @@ -2044,7 +2088,7 @@ fn remove_server_directive_index_in_module(
fn remove_server_directive_index_in_fn(
stmts: &mut Vec<Stmt>,
is_action_fn: &mut bool,
is_cache_fn: &mut bool,
cache_type: &mut Option<String>,
action_span: &mut Option<Span>,
enabled: bool,
) {
Expand Down Expand Up @@ -2097,10 +2141,18 @@ fn remove_server_directive_index_in_fn(
)
.emit();
});
} else if value == "use cache" {
} else if value == "use cache" || value.starts_with("use cache: ") {
if is_directive {
// Detect typo of "use server"
*is_cache_fn = true;
*cache_type = Some(
if value == "use cache" {
"default".into()
} else {
// Slice the value after "use cache: "
value.split_at(
"use cache: ".len(),
).1.into()
},
);
return false;
} else {
HANDLER.with(|handler| {
Expand Down
2 changes: 1 addition & 1 deletion crates/next-custom-transforms/tests/errors/server-actions/server-graph/7/output.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
x Server actions must be async functions
x Server Actions must be async functions
,-[input.js:1:1]
1 | ,-> const foo = () => {
2 | | 'use server'
Expand Down
11 changes: 11 additions & 0 deletions crates/next-custom-transforms/tests/fixture/server-actions/server/37/input.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use server'

async function fn() {
'use cache'
return 'foo'
}

async function Component() {
const data = await fn()
return <div>{data}</div>
}
Loading

0 comments on commit 5c235de

Please sign in to comment.