From 93fd9d38f640b75b0955fe1f673ca3f8e7727a4b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 12 Dec 2022 23:32:54 +0100 Subject: [PATCH] Erase dynamic ssr:false imports on server (#43974) Fixes #43895 There's a `is_server_components` condition introduced in #42426 but it's always truthy so the `ssr:false` is not erased properly. Then in #42589 the flag is removed but also the optimization is removed as well. This PR reverts the unexpected change removed in #42589 to keep the removal for client dynamic imports on server side. Add tests to keep there's no trace ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) --- packages/next-swc/crates/core/src/lib.rs | 1 + .../next-swc/crates/core/src/next_dynamic.rs | 42 ++++++++++++++++++- packages/next-swc/crates/core/tests/errors.rs | 1 + .../next-swc/crates/core/tests/fixture.rs | 3 ++ .../with-options/output-server.js | 4 +- .../wrapped-import/output-server.js | 2 +- .../basic-basepath/next-dynamic.test.ts | 9 ++++ test/development/basic/next-dynamic.test.ts | 9 ++++ 8 files changed, 66 insertions(+), 5 deletions(-) diff --git a/packages/next-swc/crates/core/src/lib.rs b/packages/next-swc/crates/core/src/lib.rs index ddc76edec38a841..32c6c02218955a2 100644 --- a/packages/next-swc/crates/core/src/lib.rs +++ b/packages/next-swc/crates/core/src/lib.rs @@ -185,6 +185,7 @@ where next_dynamic::next_dynamic( opts.is_development, opts.is_server, + opts.server_components.is_some(), file.name.clone(), opts.pages_dir.clone() ), diff --git a/packages/next-swc/crates/core/src/next_dynamic.rs b/packages/next-swc/crates/core/src/next_dynamic.rs index c8c24f6d67afab5..422e2515a9da159 100644 --- a/packages/next-swc/crates/core/src/next_dynamic.rs +++ b/packages/next-swc/crates/core/src/next_dynamic.rs @@ -5,23 +5,26 @@ use pathdiff::diff_paths; use swc_core::{ common::{errors::HANDLER, FileName, DUMMY_SP}, ecma::ast::{ - ArrayLit, ArrowExpr, BinExpr, BinaryOp, BlockStmtOrExpr, CallExpr, Callee, Expr, + ArrayLit, ArrowExpr, BinExpr, BinaryOp, BlockStmtOrExpr, Bool, CallExpr, Callee, Expr, ExprOrSpread, Id, Ident, ImportDecl, ImportSpecifier, KeyValueProp, Lit, MemberExpr, - MemberProp, ObjectLit, Prop, PropName, PropOrSpread, Str, Tpl, + MemberProp, Null, ObjectLit, Prop, PropName, PropOrSpread, Str, Tpl, }, ecma::atoms::js_word, + ecma::utils::ExprFactory, ecma::visit::{Fold, FoldWith}, }; pub fn next_dynamic( is_development: bool, is_server: bool, + is_server_components: bool, filename: FileName, pages_dir: Option, ) -> impl Fold { NextDynamicPatcher { is_development, is_server, + is_server_components, pages_dir, filename, dynamic_bindings: vec![], @@ -34,6 +37,7 @@ pub fn next_dynamic( struct NextDynamicPatcher { is_development: bool, is_server: bool, + is_server_components: bool, pages_dir: Option, filename: FileName, dynamic_bindings: Vec, @@ -229,16 +233,50 @@ impl Fold for NextDynamicPatcher { value: generated, })))]; + let mut has_ssr_false = false; + if expr.args.len() == 2 { if let Expr::Object(ObjectLit { props: options_props, .. }) = &*expr.args[1].expr { + for prop in options_props.iter() { + if let Some(KeyValueProp { key, value }) = match prop { + PropOrSpread::Prop(prop) => match &**prop { + Prop::KeyValue(key_value_prop) => Some(key_value_prop), + _ => None, + }, + _ => None, + } { + if let Some(Ident { + sym, + span: _, + optional: _, + }) = match key { + PropName::Ident(ident) => Some(ident), + _ => None, + } { + if sym == "ssr" { + if let Some(Lit::Bool(Bool { + value: false, + span: _, + })) = value.as_lit() + { + has_ssr_false = true + } + } + } + } + } props.extend(options_props.iter().cloned()); } } + if has_ssr_false && self.is_server && !self.is_server_components { + expr.args[0] = Lit::Null(Null { span: DUMMY_SP }).as_arg(); + } + let second_arg = ExprOrSpread { spread: None, expr: Box::new(Expr::Object(ObjectLit { diff --git a/packages/next-swc/crates/core/tests/errors.rs b/packages/next-swc/crates/core/tests/errors.rs index 5d2691d3f36f367..6d627c12e665e3f 100644 --- a/packages/next-swc/crates/core/tests/errors.rs +++ b/packages/next-swc/crates/core/tests/errors.rs @@ -46,6 +46,7 @@ fn next_dynamic_errors(input: PathBuf) { next_dynamic( true, false, + false, FileName::Real(PathBuf::from("/some-project/src/some-file.js")), Some("/some-project/src".into()), ) diff --git a/packages/next-swc/crates/core/tests/fixture.rs b/packages/next-swc/crates/core/tests/fixture.rs index cc7f9cd85b3342a..478f667c1ccd066 100644 --- a/packages/next-swc/crates/core/tests/fixture.rs +++ b/packages/next-swc/crates/core/tests/fixture.rs @@ -49,6 +49,7 @@ fn next_dynamic_fixture(input: PathBuf) { next_dynamic( true, false, + false, FileName::Real(PathBuf::from("/some-project/src/some-file.js")), Some("/some-project/src".into()), ) @@ -61,6 +62,7 @@ fn next_dynamic_fixture(input: PathBuf) { syntax(), &|_tr| { next_dynamic( + false, false, false, FileName::Real(PathBuf::from("/some-project/src/some-file.js")), @@ -77,6 +79,7 @@ fn next_dynamic_fixture(input: PathBuf) { next_dynamic( false, true, + false, FileName::Real(PathBuf::from("/some-project/src/some-file.js")), Some("/some-project/src".into()), ) diff --git a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js index e152280bdef1bb6..cbd15be491f9279 100644 --- a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js +++ b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js @@ -7,7 +7,7 @@ const DynamicComponentWithCustomLoading = dynamic(()=>import('../components/hell }, loading: ()=>

...

}); -const DynamicClientOnlyComponent = dynamic(()=>import('../components/hello'), { +const DynamicClientOnlyComponent = dynamic(null, { loadableGenerated: { modules: [ "some-file.js -> " + "../components/hello" @@ -15,7 +15,7 @@ const DynamicClientOnlyComponent = dynamic(()=>import('../components/hello'), { }, ssr: false }); -const DynamicClientOnlyComponentWithSuspense = dynamic(()=>import('../components/hello'), { +const DynamicClientOnlyComponentWithSuspense = dynamic(null, { loadableGenerated: { modules: [ "some-file.js -> " + "../components/hello" diff --git a/packages/next-swc/crates/core/tests/fixture/next-dynamic/wrapped-import/output-server.js b/packages/next-swc/crates/core/tests/fixture/next-dynamic/wrapped-import/output-server.js index 95b51047d831f86..df6e0e69d6ccc7e 100644 --- a/packages/next-swc/crates/core/tests/fixture/next-dynamic/wrapped-import/output-server.js +++ b/packages/next-swc/crates/core/tests/fixture/next-dynamic/wrapped-import/output-server.js @@ -1,5 +1,5 @@ import dynamic from 'next/dynamic'; -const DynamicComponent = dynamic(()=>handleImport(import('./components/hello')), { +const DynamicComponent = dynamic(null, { loadableGenerated: { modules: [ "some-file.js -> " + "./components/hello" diff --git a/test/development/basic-basepath/next-dynamic.test.ts b/test/development/basic-basepath/next-dynamic.test.ts index 1d96cb64a856718..d628233c5a82dee 100644 --- a/test/development/basic-basepath/next-dynamic.test.ts +++ b/test/development/basic-basepath/next-dynamic.test.ts @@ -133,6 +133,15 @@ describe('basic next/dynamic usage', () => { } } }) + + if (!(global as any).isNextDev) { + it('should not include ssr:false imports to server trace', async () => { + const trace = JSON.parse( + await next.readFile('.next/server/pages/dynamic/no-ssr.js.nft.json') + ) as { files: string[] } + expect(trace).not.toContain('hello1') + }) + } }) describe('ssr:true option', () => { diff --git a/test/development/basic/next-dynamic.test.ts b/test/development/basic/next-dynamic.test.ts index b26c4ecb7c7bb10..979d754f39d12f9 100644 --- a/test/development/basic/next-dynamic.test.ts +++ b/test/development/basic/next-dynamic.test.ts @@ -153,6 +153,15 @@ describe('basic next/dynamic usage', () => { } } }) + + if (!(global as any).isNextDev) { + it('should not include ssr:false imports to server trace', async () => { + const trace = JSON.parse( + await next.readFile('.next/server/pages/dynamic/no-ssr.js.nft.json') + ) as { files: string[] } + expect(trace).not.toContain('hello1') + }) + } }) describe('custom chunkfilename', () => {