Skip to content

Commit

Permalink
Change the Server Actions feature flag to be validated at compile time (
Browse files Browse the repository at this point in the history 
#52147)

Currently we are validating the `experimental.serverActions` flag when creating the actual entries for Server Actions, this causes two problems. One is that syntax errors caught at compilation time are still shown, even if you don't have this flag enabled. Another problem is we still traverse the client graph to collect these action modules even if the flag isn't enabled.

This PR moves that check to be happening at compilation time, which addresses the two above but also brings the extra benefit of showing the exact span and module trace that errors:

<img width="974" alt="CleanShot 2023-07-03 at 20 26 34@2x" src="https://github.com/vercel/next.js/assets/3676859/1676b1f6-e205-4963-bce4-5b515a698e9c">
  • Loading branch information
@shuding
shuding committed Jul 3, 2023
1 parent cd9372c commit 3a87f00
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 74 deletions.
25 changes: 25 additions & 0 deletions packages/next-swc/crates/core/src/server_actions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use turbopack_binding::swc::core::{
#[serde(deny_unknown_fields, rename_all = "camelCase")]
pub struct Config {
pub is_server: bool,
pub enabled: bool
}

pub fn server_actions<C: Comments>(
Expand Down Expand Up @@ -101,6 +102,7 @@ impl<C: Comments> ServerActions<C> {
&mut body.stmts,
remove_directive,
&mut is_action_fn,
self.config.enabled,
);

if is_action_fn && !self.config.is_server {
Expand Down Expand Up @@ -721,6 +723,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
stmts,
&mut self.in_action_file,
&mut self.has_action,
self.config.enabled,
);

let old_annotations = self.annotations.take();
Expand Down Expand Up @@ -1231,6 +1234,7 @@ fn remove_server_directive_index_in_module(
stmts: &mut Vec<ModuleItem>,
in_action_file: &mut bool,
has_action: &mut bool,
enabled: bool,
) {
let mut is_directive = true;

Expand All @@ -1244,6 +1248,16 @@ fn remove_server_directive_index_in_module(
if is_directive {
*in_action_file = true;
*has_action = true;
if !enabled {
HANDLER.with(|handler| {
handler
.struct_span_err(
*span,
"To use Server Actions, please enable the feature flag in your Next.js config. Read more: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#convention",
)
.emit()
});
}
return false;
} else {
HANDLER.with(|handler| {
Expand Down Expand Up @@ -1320,6 +1334,7 @@ fn remove_server_directive_index_in_fn(
stmts: &mut Vec<Stmt>,
remove_directive: bool,
is_action_fn: &mut bool,
enabled: bool,
) {
let mut is_directive = true;

Expand All @@ -1332,6 +1347,16 @@ fn remove_server_directive_index_in_fn(
if value == "use server" {
if is_directive {
*is_action_fn = true;
if !enabled {
HANDLER.with(|handler| {
handler
.struct_span_err(
*span,
"To use Server Actions, please enable the feature flag in your Next.js config. Read more: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#convention",
)
.emit()
});
}
if remove_directive {
return false;
}
Expand Down
7 changes: 7 additions & 0 deletions packages/next/src/build/swc/options.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ function getBaseSWCOptions({
swcCacheDir,
isServerLayer,
hasServerComponents,
isServerActionsEnabled,
}: {
filename: string
jest?: boolean
Expand All @@ -57,6 +58,7 @@ function getBaseSWCOptions({
swcCacheDir?: string
isServerLayer?: boolean
hasServerComponents?: boolean
isServerActionsEnabled?: boolean
}) {
const parserConfig = getParserOptions({ filename, jsConfig })
const paths = jsConfig?.compilerOptions?.paths
Expand Down Expand Up @@ -157,6 +159,8 @@ function getBaseSWCOptions({
: undefined,
serverActions: hasServerComponents
? {
// TODO-APP: When Server Actions is stable, we need to remove this flag.
enabled: !!isServerActionsEnabled,
isServer: !!isServerLayer,
}
: undefined,
Expand Down Expand Up @@ -285,6 +289,7 @@ export function getLoaderSWCOptions({
relativeFilePathFromRoot,
hasServerComponents,
isServerLayer,
isServerActionsEnabled,
}: // This is not passed yet as "paths" resolving is handled by webpack currently.
// resolvedBaseUrl,
{
Expand All @@ -304,6 +309,7 @@ export function getLoaderSWCOptions({
relativeFilePathFromRoot: string
hasServerComponents?: boolean
isServerLayer: boolean
isServerActionsEnabled?: boolean
}) {
let baseOptions: any = getBaseSWCOptions({
filename,
Expand All @@ -318,6 +324,7 @@ export function getLoaderSWCOptions({
swcCacheDir,
hasServerComponents,
isServerLayer,
isServerActionsEnabled,
})
baseOptions.fontLoaders = {
fontLoaders: [
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/build/webpack/loaders/next-swc-loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ async function loaderTransform(
swcCacheDir,
relativeFilePathFromRoot,
hasServerComponents,
isServerActionsEnabled: nextConfig?.experimental?.serverActions,
isServerLayer,
})

Expand Down
135 changes: 62 additions & 73 deletions packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,21 +350,13 @@ export class FlightClientEntryPlugin {
)

if (actionEntryImports.size > 0) {
if (!this.useServerActions) {
compilation.errors.push(
new Error(
'Server Actions require `experimental.serverActions` option to be enabled in your Next.js config: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions'
)
)
} else {
if (!actionMapsPerEntry[name]) {
actionMapsPerEntry[name] = new Map()
}
actionMapsPerEntry[name] = new Map([
...actionMapsPerEntry[name],
...actionEntryImports,
])
if (!actionMapsPerEntry[name]) {
actionMapsPerEntry[name] = new Map()
}
actionMapsPerEntry[name] = new Map([
...actionMapsPerEntry[name],
...actionEntryImports,
])
}
})

Expand All @@ -388,31 +380,28 @@ export class FlightClientEntryPlugin {
)
}

compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, () => {
const addedClientActionEntryList: Promise<any>[] = []
const actionMapsPerClientEntry: Record<string, Map<string, string[]>> = {}

// We need to create extra action entries that are created from the
// client layer.
// Start from each entry's created SSR dependency from our previous step.
for (const [name, ssrEntryDepdendencies] of Object.entries(
createdSSRDependenciesForEntry
)) {
// Collect from all entries, e.g. layout.js, page.js, loading.js, ...
// add agregate them.
const actionEntryImports = this.collectClientActionsFromDependencies({
compilation,
dependencies: ssrEntryDepdendencies,
})
if (this.useServerActions) {
compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, () => {
const addedClientActionEntryList: Promise<any>[] = []
const actionMapsPerClientEntry: Record<
string,
Map<string, string[]>
> = {}

// We need to create extra action entries that are created from the
// client layer.
// Start from each entry's created SSR dependency from our previous step.
for (const [name, ssrEntryDepdendencies] of Object.entries(
createdSSRDependenciesForEntry
)) {
// Collect from all entries, e.g. layout.js, page.js, loading.js, ...
// add agregate them.
const actionEntryImports = this.collectClientActionsFromDependencies({
compilation,
dependencies: ssrEntryDepdendencies,
})

if (actionEntryImports.size > 0) {
if (!this.useServerActions) {
compilation.errors.push(
new Error(
'Server Actions require `experimental.serverActions` option to be enabled in your Next.js config: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions'
)
)
} else {
if (actionEntryImports.size > 0) {
if (!actionMapsPerClientEntry[name]) {
actionMapsPerClientEntry[name] = new Map()
}
Expand All @@ -422,47 +411,47 @@ export class FlightClientEntryPlugin {
])
}
}
}

for (const [name, actionEntryImports] of Object.entries(
actionMapsPerClientEntry
)) {
// If an action method is already created in the server layer, we don't
// need to create it again in the action layer.
// This is to avoid duplicate action instances and make sure the module
// state is shared.
let remainingClientImportedActions = false
const remainingActionEntryImports = new Map<string, string[]>()
for (const [dep, actionNames] of actionEntryImports) {
const remainingActionNames = []
for (const actionName of actionNames) {
const id = name + '@' + dep + '@' + actionName
if (!createdActions.has(id)) {
remainingActionNames.push(actionName)
for (const [name, actionEntryImports] of Object.entries(
actionMapsPerClientEntry
)) {
// If an action method is already created in the server layer, we don't
// need to create it again in the action layer.
// This is to avoid duplicate action instances and make sure the module
// state is shared.
let remainingClientImportedActions = false
const remainingActionEntryImports = new Map<string, string[]>()
for (const [dep, actionNames] of actionEntryImports) {
const remainingActionNames = []
for (const actionName of actionNames) {
const id = name + '@' + dep + '@' + actionName
if (!createdActions.has(id)) {
remainingActionNames.push(actionName)
}
}
if (remainingActionNames.length > 0) {
remainingActionEntryImports.set(dep, remainingActionNames)
remainingClientImportedActions = true
}
}
if (remainingActionNames.length > 0) {
remainingActionEntryImports.set(dep, remainingActionNames)
remainingClientImportedActions = true
}
}

if (remainingClientImportedActions) {
addedClientActionEntryList.push(
this.injectActionEntry({
compiler,
compilation,
actions: remainingActionEntryImports,
entryName: name,
bundlePath: name,
fromClient: true,
})
)
if (remainingClientImportedActions) {
addedClientActionEntryList.push(
this.injectActionEntry({
compiler,
compilation,
actions: remainingActionEntryImports,
entryName: name,
bundlePath: name,
fromClient: true,
})
)
}
}
}

return Promise.all(addedClientActionEntryList)
})
return Promise.all(addedClientActionEntryList)
})
}

// Invalidate in development to trigger recompilation
const invalidator = getInvalidator(compiler.outputPath)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app-dir/actions/app-action-invalid.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ createNextDescribe(

it('should error if serverActions is not enabled', async () => {
expect(next.cliOutput).toContain(
'Server Actions require `experimental.serverActions` option'
'To use Server Actions, please enable the feature flag in your Next.js config.'
)
})
}
Expand Down

0 comments on commit 3a87f00

Please sign in to comment.