Skip to content

Commit

Permalink
Allow server-only in server targets and client-only in client compone…
Browse files Browse the repository at this point in the history
…nts targets to be available (#55394)

Users want to use `server-only` to restrict the middleware / app routes / pages api, but now it's failing as we're treating them as different webpack layers, but validating the `server-only` only with server components layers.

Here we modify the rules a bit to let everyone can use "server-only" for the bundles that targeting server-side.

For next-swc transformer, we introduce the new option `bundleType` which only has `"server" | "client" | "default"` 3 values:
* - `server`  for server-side targets, like server components, app routes, pages api, middleware
* - `client`   for client components targets such as client components app pages, or page routes under pages directory.
* - `default` for environment like jest, we don't validate module graph with swc, replaced the `disable_checks` introduced  [#54891](#54891).

Refactor a bit webpack-config to adapt to the new rules, after that `server-only` will be able to used in the server-side targets conventions like middleware and `pages/api`

Fixes #43700
Fixes #54549
Fixes #52833

Closes NEXT-1616
Closes NEXT-1607
Closes NEXT-1385
  • Loading branch information
huozhi authored Sep 18, 2023
1 parent b2facf5 commit ffc0e54
Show file tree
Hide file tree
Showing 22 changed files with 319 additions and 111 deletions.
7 changes: 4 additions & 3 deletions packages/next-swc/crates/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ use turbopack_binding::swc::{
SyntaxContext,
},
ecma::{
ast::EsVersion, parser::parse_file_as_module, transforms::base::pass::noop, visit::Fold,
ast::EsVersion, atoms::JsWord, parser::parse_file_as_module,
transforms::base::pass::noop, visit::Fold,
},
},
custom_transform::modularize_imports,
Expand Down Expand Up @@ -97,7 +98,7 @@ pub struct TransformOptions {
pub is_server: bool,

#[serde(default)]
pub disable_checks: bool,
pub bundle_target: JsWord,

#[serde(default)]
pub server_components: Option<react_server_components::Config>,
Expand Down Expand Up @@ -198,7 +199,7 @@ where
config.clone(),
comments.clone(),
opts.app_dir.clone(),
opts.disable_checks
opts.bundle_target.clone()
)),
_ => Either::Right(noop()),
},
Expand Down
38 changes: 20 additions & 18 deletions packages/next-swc/crates/core/src/react_server_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ struct ReactServerComponents<C: Comments> {
invalid_client_imports: Vec<JsWord>,
invalid_server_react_apis: Vec<JsWord>,
invalid_server_react_dom_apis: Vec<JsWord>,
disable_checks: bool,
bundle_target: String,
}

struct ModuleImports {
Expand All @@ -67,15 +67,25 @@ impl<C: Comments> VisitMut for ReactServerComponents<C> {
let is_cjs = contains_cjs(module);

if self.is_server {
if !is_client_entry {
self.assert_server_graph(&imports, module);
} else {
if is_client_entry {
self.to_module_ref(module, is_cjs);
return;
} else if self.bundle_target == "server" {
// Only assert server graph if file's bundle target is "server", e.g.
// * server components pages
// * pages bundles on SSR layer
// * middleware
// * app/pages api routes
self.assert_server_graph(&imports, module);
}
} else {
if !is_action_file {
self.assert_client_graph(&imports, module);
// Only assert client graph if the file is not an action file,
// and bundle target is "client" e.g.
// * client components pages
// * pages bundles on browser layer
if !is_action_file && self.bundle_target == "client" {
self.assert_client_graph(&imports);
self.assert_invalid_api(module, true);
}
if is_client_entry {
self.prepend_comment_node(module, is_cjs);
Expand Down Expand Up @@ -129,7 +139,7 @@ impl<C: Comments> ReactServerComponents<C> {
if is_action_file {
panic_both_directives(expr_stmt.span)
}
} else if !self.disable_checks {
} else if self.bundle_target != "default" {
HANDLER.with(|handler| {
handler
.struct_span_err(
Expand Down Expand Up @@ -334,9 +344,6 @@ impl<C: Comments> ReactServerComponents<C> {
}

fn assert_server_graph(&self, imports: &[ModuleImports], module: &Module) {
if self.disable_checks {
return;
}
for import in imports {
let source = import.source.0.clone();
if self.invalid_server_imports.contains(&source) {
Expand Down Expand Up @@ -408,10 +415,7 @@ impl<C: Comments> ReactServerComponents<C> {
}
}

fn assert_client_graph(&self, imports: &[ModuleImports], module: &Module) {
if self.disable_checks {
return;
}
fn assert_client_graph(&self, imports: &[ModuleImports]) {
for import in imports {
let source = import.source.0.clone();
if self.invalid_client_imports.contains(&source) {
Expand All @@ -425,8 +429,6 @@ impl<C: Comments> ReactServerComponents<C> {
})
}
}

self.assert_invalid_api(module, true);
}

fn assert_invalid_api(&self, module: &Module, is_client_entry: bool) {
Expand Down Expand Up @@ -567,17 +569,17 @@ pub fn server_components<C: Comments>(
config: Config,
comments: C,
app_dir: Option<PathBuf>,
disable_checks: bool,
bundle_target: JsWord,
) -> impl Fold + VisitMut {
let is_server: bool = match &config {
Config::WithOptions(x) => x.is_server,
_ => true,
};
as_folder(ReactServerComponents {
disable_checks,
is_server,
comments,
filepath: filename.to_string(),
bundle_target: bundle_target.to_string(),
app_dir,
export_names: vec![],
invalid_server_imports: vec![
Expand Down
8 changes: 4 additions & 4 deletions packages/next-swc/crates/core/tests/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ fn react_server_components_server_graph_errors(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
false,
String::from("server").into(),
)
},
&input,
Expand All @@ -122,7 +122,7 @@ fn react_server_components_client_graph_errors(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
false,
String::from("client").into(),
)
},
&input,
Expand Down Expand Up @@ -169,7 +169,7 @@ fn react_server_actions_server_errors(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
false
String::from("default").into(),
),
server_actions(
&FileName::Real("/app/item.js".into()),
Expand Down Expand Up @@ -205,7 +205,7 @@ fn react_server_actions_client_errors(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
false
String::from("client").into(),
),
server_actions(
&FileName::Real("/app/item.js".into()),
Expand Down
8 changes: 4 additions & 4 deletions packages/next-swc/crates/core/tests/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ fn react_server_components_server_graph_fixture(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
false,
String::from("default").into(),
)
},
&input,
Expand All @@ -348,7 +348,7 @@ fn react_server_components_no_checks_server_graph_fixture(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
true,
String::from("default").into(),
)
},
&input,
Expand All @@ -370,7 +370,7 @@ fn react_server_components_client_graph_fixture(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
false,
String::from("default").into(),
)
},
&input,
Expand All @@ -392,7 +392,7 @@ fn react_server_components_no_checks_client_graph_fixture(input: PathBuf) {
),
tr.comments.as_ref().clone(),
None,
true,
String::from("default").into(),
)
},
&input,
Expand Down
2 changes: 1 addition & 1 deletion packages/next-swc/crates/core/tests/full.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn test(input: &Path, minify: bool) {
auto_modularize_imports: None,
optimize_barrel_exports: None,
optimize_server_react: None,
disable_checks: false,
bundle_target: String::from("default").into(),
};

let unresolved_mark = Mark::new();
Expand Down
12 changes: 10 additions & 2 deletions packages/next/src/build/swc/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import type {
StyledComponentsConfig,
} from '../../server/config-shared'

type BundleType = 'client' | 'server' | 'default'

const nextDistPath =
/(next[\\/]dist[\\/]shared[\\/]lib)|(next[\\/]dist[\\/]client)|(next[\\/]dist[\\/]pages)/

Expand Down Expand Up @@ -42,6 +44,7 @@ function getBaseSWCOptions({
jsConfig,
swcCacheDir,
isServerLayer,
bundleTarget,
hasServerComponents,
isServerActionsEnabled,
}: {
Expand All @@ -55,6 +58,7 @@ function getBaseSWCOptions({
compilerOptions: NextConfig['compiler']
resolvedBaseUrl?: string
jsConfig: any
bundleTarget: BundleType
swcCacheDir?: string
isServerLayer?: boolean
hasServerComponents?: boolean
Expand Down Expand Up @@ -184,7 +188,7 @@ function getBaseSWCOptions({
isServer: !!isServerLayer,
}
: undefined,
disableChecks: false,
bundleTarget,
}
}

Expand Down Expand Up @@ -274,13 +278,14 @@ export function getJestSWCOptions({
resolvedBaseUrl,
// Don't apply server layer transformations for Jest
isServerLayer: false,
// Disable server / client graph assertions for Jest
bundleTarget: 'default',
})

const isNextDist = nextDistPath.test(filename)

return {
...baseOptions,
disableChecks: true,
env: {
targets: {
// Targets the current version of Node.js
Expand Down Expand Up @@ -317,6 +322,7 @@ export function getLoaderSWCOptions({
isServerLayer,
isServerActionsEnabled,
optimizeBarrelExports,
bundleTarget = 'client',
}: // This is not passed yet as "paths" resolving is handled by webpack currently.
// resolvedBaseUrl,
{
Expand All @@ -338,6 +344,7 @@ export function getLoaderSWCOptions({
supportedBrowsers: string[]
swcCacheDir: string
relativeFilePathFromRoot: string
bundleTarget: BundleType
hasServerComponents?: boolean
isServerLayer: boolean
isServerActionsEnabled?: boolean
Expand All @@ -357,6 +364,7 @@ export function getLoaderSWCOptions({
hasServerComponents,
isServerLayer,
isServerActionsEnabled,
bundleTarget,
})
baseOptions.fontLoaders = {
fontLoaders: [
Expand Down
14 changes: 14 additions & 0 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
MiddlewareManifest,
} from './webpack/plugins/middleware-plugin'
import type { StaticGenerationAsyncStorage } from '../client/components/static-generation-async-storage.external'
import type { WebpackLayerName } from '../lib/constants'

import '../server/require-hook'
import '../server/node-polyfill-fetch'
Expand All @@ -35,6 +36,7 @@ import {
SERVER_PROPS_SSG_CONFLICT,
MIDDLEWARE_FILENAME,
INSTRUMENTATION_HOOK_FILENAME,
WEBPACK_LAYERS,
} from '../lib/constants'
import { MODERN_BROWSERSLIST_TARGET } from '../shared/lib/constants'
import prettyBytes from '../lib/pretty-bytes'
Expand Down Expand Up @@ -2116,3 +2118,15 @@ export function getSupportedBrowsers(
// Uses modern browsers as the default.
return MODERN_BROWSERSLIST_TARGET
}

export function isWebpackServerLayer(
layer: WebpackLayerName | null | undefined
): boolean {
return Boolean(layer && WEBPACK_LAYERS.GROUP.server.includes(layer as any))
}

export function isWebpackDefaultLayer(
layer: WebpackLayerName | null | undefined
): boolean {
return layer === null || layer === undefined
}
Loading

0 comments on commit ffc0e54

Please sign in to comment.