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)
vercel · Dec 12, 2022 · 8e04398 · 8e04398
1 parent 5e3bb45
commit 8e04398
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 5 deletions.
diff --git 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
@@ -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<PathBuf>,
 ) -> 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<PathBuf>,
     filename: FileName,
     dynamic_bindings: Vec<Id>,
@@ -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
@@ -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
@@ -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
@@ -7,15 +7,15 @@ const DynamicComponentWithCustomLoading = dynamic(()=>import('../components/hell
     },
     loading: ()=><p >...</p>
 });
-const DynamicClientOnlyComponent = dynamic(()=>import('../components/hello'), {
+const DynamicClientOnlyComponent = dynamic(null, {
     loadableGenerated: {
         modules: [
             "some-file.js -> " + "../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
@@ -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
@@ -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
@@ -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', () => {