Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dynamic transform ssr:false case for pages router with ESM #59246

Merged
merged 3 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/next-swc/crates/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ pub struct TransformOptions {
#[serde(default)]
pub is_server_compiler: bool,

#[serde(default)]
pub prefer_esm: bool,

#[serde(default)]
pub server_components: Option<react_server_components::Config>,

Expand Down Expand Up @@ -238,6 +241,7 @@ where
},
_ => false,
},
opts.prefer_esm,
NextDynamicMode::Webpack,
file.name.clone(),
opts.pages_dir.clone()
Expand Down
1 change: 1 addition & 0 deletions packages/next-swc/crates/core/tests/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ fn next_dynamic_errors(input: PathBuf) {
true,
false,
false,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand Down
7 changes: 7 additions & 0 deletions packages/next-swc/crates/core/tests/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ fn next_dynamic_fixture(input: PathBuf) {
true,
false,
false,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand All @@ -81,6 +82,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
false,
false,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand All @@ -97,6 +99,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
true,
false,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand Down Expand Up @@ -124,6 +127,7 @@ fn app_dir_next_dynamic_fixture(input: PathBuf) {
true,
false,
true,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand All @@ -140,6 +144,7 @@ fn app_dir_next_dynamic_fixture(input: PathBuf) {
false,
false,
true,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand All @@ -156,6 +161,7 @@ fn app_dir_next_dynamic_fixture(input: PathBuf) {
false,
true,
true,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand All @@ -172,6 +178,7 @@ fn app_dir_next_dynamic_fixture(input: PathBuf) {
false,
true,
false,
false,
NextDynamicMode::Webpack,
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(async ()=>{
typeof require.resolveWeak !== "undefined" && require.resolveWeak("../text-dynamic-no-ssr-server");
}, {
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(async ()=>{
typeof require.resolveWeak !== "undefined" && require.resolveWeak("../text-dynamic-no-ssr-server");
}, {
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,15 @@ const DynamicComponentWithCustomLoading = dynamic(()=>import('../components/hell
},
loading: ()=><p>...</p>
});
const DynamicClientOnlyComponent = dynamic(async ()=>{
typeof require.resolveWeak !== "undefined" && require.resolveWeak("../components/hello");
}, {
const DynamicClientOnlyComponent = dynamic(()=>import('../components/hello'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
]
},
ssr: false
});
const DynamicClientOnlyComponentWithSuspense = dynamic(async ()=>{
typeof require.resolveWeak !== "undefined" && require.resolveWeak("../components/hello");
}, {
const DynamicClientOnlyComponentWithSuspense = dynamic(()=>import('../components/hello'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import dynamic from 'next/dynamic';
const DynamicComponent = dynamic(async ()=>{
typeof require.resolveWeak !== "undefined" && require.resolveWeak("./components/hello");
}, {
const DynamicComponent = dynamic(()=>handleImport(import('./components/hello')), {
loadableGenerated: {
modules: [
"some-file.js -> " + "./components/hello"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ impl CustomTransformer for NextJsDynamic {
},
self.is_server_compiler,
self.is_react_server_layer,
false,
NextDynamicMode::Webpack,
FileName::Real(ctx.file_path_str.into()),
self.pages_dir.clone(),
Expand Down
7 changes: 7 additions & 0 deletions packages/next-swc/crates/next-transform-dynamic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub fn next_dynamic(
is_development: bool,
is_server_compiler: bool,
is_react_server_layer: bool,
prefer_esm: bool,
mode: NextDynamicMode,
filename: FileName,
pages_dir: Option<PathBuf>,
Expand All @@ -36,6 +37,7 @@ pub fn next_dynamic(
is_development,
is_server_compiler,
is_react_server_layer,
prefer_esm,
pages_dir,
filename,
dynamic_bindings: vec![],
Expand Down Expand Up @@ -81,6 +83,7 @@ struct NextDynamicPatcher {
is_development: bool,
is_server_compiler: bool,
is_react_server_layer: bool,
prefer_esm: bool,
pages_dir: Option<PathBuf>,
filename: FileName,
dynamic_bindings: Vec<Id>,
Expand Down Expand Up @@ -368,6 +371,10 @@ impl Fold for NextDynamicPatcher {
if has_ssr_false
&& self.is_server_compiler
&& !self.is_react_server_layer
// When it's not prefer to picking up ESM, as it's in the pages router, we don't need to do it as it doesn't need to enter the non-ssr module.
// Also transforming it to `require.resolveWeak` and with ESM import, like require.resolveWeak(esm asset) is not available as it's commonjs importing ESM.
&& self.prefer_esm

// Only use `require.resolveWebpack` to decouple modules for webpack,
// turbopack doesn't need this
&& self.state == NextDynamicPatcherState::Webpack
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ fn next_dynamic_errors_run(input: &Path, output: &str, mode: NextDynamicMode) {
true,
false,
false,
false,
mode.clone(),
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ fn next_dynamic_fixture(input: PathBuf) {
true,
false,
false,
false,
NextDynamicMode::Webpack,
);
next_dynamic_fixture_run(
Expand All @@ -33,6 +34,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
false,
false,
false,
NextDynamicMode::Webpack,
);
next_dynamic_fixture_run(
Expand All @@ -41,6 +43,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
true,
false,
false,
NextDynamicMode::Webpack,
);

Expand All @@ -50,6 +53,7 @@ fn next_dynamic_fixture(input: PathBuf) {
true,
false,
false,
false,
NextDynamicMode::Turbopack {
dynamic_transition_name: "next-client-chunks".into(),
},
Expand All @@ -60,6 +64,7 @@ fn next_dynamic_fixture(input: PathBuf) {
true,
true,
false,
false,
NextDynamicMode::Turbopack {
dynamic_transition_name: "next-client-chunks".into(),
},
Expand All @@ -70,6 +75,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
false,
false,
false,
NextDynamicMode::Turbopack {
dynamic_transition_name: "next-dynamic".into(),
},
Expand All @@ -80,6 +86,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
true,
false,
false,
NextDynamicMode::Turbopack {
dynamic_transition_name: "next-dynamic".into(),
},
Expand All @@ -90,6 +97,7 @@ fn next_dynamic_fixture(input: PathBuf) {
false,
true,
true,
true,
NextDynamicMode::Turbopack {
dynamic_transition_name: "next-dynamic".into(),
},
Expand All @@ -102,6 +110,7 @@ fn next_dynamic_fixture_run(
is_development: bool,
is_server_compiler: bool,
is_react_server_layer: bool,
prefer_esm: bool,
mode: NextDynamicMode,
) {
let output = input.parent().unwrap().join(output);
Expand All @@ -112,6 +121,7 @@ fn next_dynamic_fixture_run(
is_development,
is_server_compiler,
is_react_server_layer,
prefer_esm,
mode.clone(),
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
Expand Down
8 changes: 8 additions & 0 deletions packages/next/src/build/swc/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function getBaseSWCOptions({
development,
hasReactRefresh,
globalWindow,
esm,
modularizeImports,
swcPlugins,
compilerOptions,
Expand All @@ -50,6 +51,7 @@ function getBaseSWCOptions({
development: boolean
hasReactRefresh: boolean
globalWindow: boolean
esm: boolean
modularizeImports?: NextConfig['modularizeImports']
compilerOptions: NextConfig['compiler']
swcPlugins: ExperimentalConfig['swcPlugins']
Expand Down Expand Up @@ -188,6 +190,9 @@ function getBaseSWCOptions({
isReactServerLayer: !!isReactServerLayer,
}
: undefined,
// For app router we prefer to bundle ESM,
// On server side of pages router we prefer CJS.
preferEsm: esm,
}
}

Expand Down Expand Up @@ -289,6 +294,7 @@ export function getJestSWCOptions({
compilerOptions,
jsConfig,
resolvedBaseUrl,
esm,
// Don't apply server layer transformations for Jest
isReactServerLayer: false,
// Disable server / client graph assertions for Jest
Expand Down Expand Up @@ -372,6 +378,7 @@ export function getLoaderSWCOptions({
swcCacheDir,
isReactServerLayer,
serverComponents,
esm: !!esm,
})
baseOptions.fontLoaders = {
fontLoaders: [
Expand Down Expand Up @@ -423,6 +430,7 @@ export function getLoaderSWCOptions({
isServerCompiler: isServer,
pagesDir,
appDir,
preferEsm: !!esm,
isPageFile,
env: {
targets: {
Expand Down
19 changes: 19 additions & 0 deletions test/development/basic/next-dynamic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,25 @@ describe.each([
}
}
})

it('should import and render the ESM module correctly on client side', async () => {
let browser
try {
browser = await webdriver(
next.url,
basePath + '/dynamic/no-ssr-esm'
)
await check(
() => browser.elementByCss('body').text(),
/esm.mjs/
)
expect(await hasRedbox(browser, false)).toBe(false)
} finally {
if (browser) {
await browser.close()
}
}
})
})

describe('ssr:true option', () => {
Expand Down
7 changes: 7 additions & 0 deletions test/development/basic/next-dynamic/components/esm.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import React from 'react'

window.ua = navigator.userAgent

export default function Component() {
return <p>esm.mjs</p>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import dynamic from 'next/dynamic'

const Page = dynamic(() => import('../../components/esm.mjs'), { ssr: false })

export default Page