feat(next-swc): lint for ssr: false in server components (vercel#70378

)

Co-authored-by: Jiachi Liu <inbox@huozhi.im>

crates/next-custom-transforms/src/transforms/react_server_components.rs

@@ -21,7 +21,7 @@ use swc_core::{
     },
 };
 
-use super::cjs_finder::contains_cjs;
+use super::{cjs_finder::contains_cjs, import_analyzer::ImportMap};
 
 #[derive(Clone, Debug, Deserialize)]
 #[serde(untagged)]
@@ -76,6 +76,7 @@ enum RSCErrorKind {
     NextRscErrConflictMetadataExport(Span),
     NextRscErrInvalidApi((String, Span)),
     NextRscErrDeprecatedApi((String, String, Span)),
+    NextSsrDynamicFalseNotAllowed(Span),
 }
 
 impl<C: Comments> VisitMut for ReactServerComponents<C> {
@@ -301,6 +302,11 @@ fn report_error(app_dir: &Option<PathBuf>, filepath: &str, error_kind: RSCErrorK
             ),
             _ => (format!("\"{source}\" is deprecated."), span),
         },
+        RSCErrorKind::NextSsrDynamicFalseNotAllowed(span) => (
+            "`ssr: false` is not allowed with `next/dynamic` in Server Components. Please move it into a client component."
+                .to_string(),
+            span,
+        ),
     };
 
     HANDLER.with(|handler| handler.struct_span_err(span, msg.as_str()).emit())
@@ -502,6 +508,7 @@ struct ReactServerComponentValidator {
     invalid_client_imports: Vec<JsWord>,
     invalid_client_lib_apis_mapping: HashMap<&'static str, Vec<&'static str>>,
     pub directive_import_collection: Option<(bool, bool, RcVec<ModuleImports>, RcVec<String>)>,
+    imports: ImportMap,
 }
 
 // A type to workaround a clippy warning.
@@ -576,6 +583,7 @@ impl ReactServerComponentValidator {
             invalid_client_imports: vec![JsWord::from("server-only"), JsWord::from("next/headers")],
 
             invalid_client_lib_apis_mapping: [("next/server", vec!["unstable_after"])].into(),
+            imports: ImportMap::default(),
         }
     }
 
@@ -584,6 +592,13 @@ impl ReactServerComponentValidator {
         RE.is_match(filepath)
     }
 
+    fn is_callee_next_dynamic(&self, callee: &Callee) -> bool {
+        match callee {
+            Callee::Expr(expr) => self.imports.is_import(expr, "next/dynamic", "default"),
+            _ => false,
+        }
+    }
+
     // Asserts the server lib apis
     // e.g.
     // assert_invalid_server_lib_apis("react", import)
@@ -817,6 +832,44 @@ impl ReactServerComponentValidator {
             }
         }
     }
+
+    /// ```
+    /// import dynamic from 'next/dynamic'
+    ///
+    /// dynamic(() => import(...)) // ✅
+    /// dynamic(() => import(...), { ssr: true }) // ✅
+    /// dynamic(() => import(...), { ssr: false }) // ❌
+    /// ```
+
+    fn check_for_next_ssr_false(&self, node: &CallExpr) -> Option<()> {
+        if !self.is_callee_next_dynamic(&node.callee) {
+            return None;
+        }
+
+        let ssr_arg = node.args.get(1)?;
+        let obj = ssr_arg.expr.as_object()?;
+
+        for prop in obj.props.iter().filter_map(|v| v.as_prop()?.as_key_value()) {
+            let is_ssr = match &prop.key {
+                PropName::Ident(IdentName { sym, .. }) => sym == "ssr",
+                PropName::Str(s) => s.value == "ssr",
+                _ => false,
+            };
+
+            if is_ssr {
+                let value = prop.value.as_lit()?;
+                if let Lit::Bool(Bool { value: false, .. }) = value {
+                    report_error(
+                        &self.app_dir,
+                        &self.filepath,
+                        RSCErrorKind::NextSsrDynamicFalseNotAllowed(node.span),
+                    );
+                }
+            }
+        }
+
+        None
+    }
 }
 
 impl Visit for ReactServerComponentValidator {
@@ -830,7 +883,17 @@ impl Visit for ReactServerComponentValidator {
         }
     }
 
+    fn visit_call_expr(&mut self, node: &CallExpr) {
+        node.visit_children_with(self);
+
+        if self.is_react_server_layer {
+            self.check_for_next_ssr_false(node);
+        }
+    }
+
     fn visit_module(&mut self, module: &Module) {
+        self.imports = ImportMap::analyze(module);
+
         let (is_client_entry, is_action_file, imports, export_names) =
             collect_top_level_directives_and_imports(&self.app_dir, &self.filepath, module);
         let imports = Rc::new(imports);

crates/next-custom-transforms/tests/errors/react-server-components/server-graph/dynamic-ssr-false/input.js

@@ -0,0 +1,5 @@
+import dynamic from 'next/dynamic'
+
+export default function () {
+  return dynamic(() => import('client-only'), { ssr: false })
+}

crates/next-custom-transforms/tests/errors/react-server-components/server-graph/dynamic-ssr-false/output.js

@@ -0,0 +1,6 @@
+import dynamic from 'next/dynamic';
+export default function() {
+    return dynamic(()=>import('client-only'), {
+        ssr: false
+    });
+}

crates/next-custom-transforms/tests/errors/react-server-components/server-graph/dynamic-ssr-false/output.stderr

@@ -0,0 +1,7 @@
+  x `ssr: false` is not allowed with `next/dynamic` in Server Components. Please move it into a client component.
+   ,-[input.js:4:1]
+ 3 | export default function () {
+ 4 |   return dynamic(() => import('client-only'), { ssr: false })
+   :          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ 5 | }
+   `----

docs/02-app/01-building-your-application/06-optimizing/07-lazy-loading.mdx

@@ -72,8 +72,6 @@ const ComponentC = dynamic(() => import('../components/C'), { ssr: false })
 If you dynamically import a Server Component, only the Client Components that are children of the Server Component will be lazy-loaded - not the Server Component itself.
 It will also help preload the static assets such as CSS when you're using it in Server Components.
 
-> **Note:** `ssr: false` option is not supported in Server Components.
-
 ```jsx filename="app/page.js"
 import dynamic from 'next/dynamic'
 
@@ -89,6 +87,9 @@ export default function ServerComponentExample() {
 }
 ```
 
+> **Note:** `ssr: false` option is not supported in Server Components. You will see an error if you try to use it in Server Components.
+> `ssr: false` is not allowed with `next/dynamic` in Server Components. Please move it into a client component.
+
 ### Loading External Libraries
 
 External libraries can be loaded on demand using the [`import()`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Operators/import) function. This example uses the external library `fuse.js` for fuzzy search. The module is only loaded on the client after the user types in the search input.

test/development/app-dir/server-component-next-dynamic-ssr-false/app/client.js

@@ -0,0 +1,5 @@
+'use client'
+
+export default function Client() {
+  return 'client'
+}

test/development/app-dir/server-component-next-dynamic-ssr-false/app/layout.js

@@ -0,0 +1,7 @@
+export default function Root({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

test/development/app-dir/server-component-next-dynamic-ssr-false/app/page.js

@@ -0,0 +1,7 @@
+import dynamic from 'next/dynamic'
+
+const DynamicClient = dynamic(() => import('./client'), { ssr: false })
+
+export default function Page() {
+  return <DynamicClient />
+}

test/development/app-dir/server-component-next-dynamic-ssr-false/server-component-next-dynamic-ssr-false.test.ts

@@ -0,0 +1,52 @@
+import { nextTestSetup } from 'e2e-utils'
+import {
+  assertHasRedbox,
+  getRedboxDescription,
+  getRedboxSource,
+} from 'next-test-utils'
+
+describe('app-dir - server-component-next-dynamic-ssr-false', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it('should error when use dynamic ssr:false in server component', async () => {
+    const browser = await next.browser('/')
+    await assertHasRedbox(browser)
+    const redbox = {
+      description: await getRedboxDescription(browser),
+      source: await getRedboxSource(browser),
+    }
+
+    expect(redbox.description).toBe('Failed to compile')
+    if (process.env.TURBOPACK) {
+      expect(redbox.source).toMatchInlineSnapshot(`
+        "./app/page.js:3:23
+        Ecmascript file had an error
+          1 | import dynamic from 'next/dynamic'
+          2 |
+        > 3 | const DynamicClient = dynamic(() => import('./client'), { ssr: false })
+            |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+          4 |
+          5 | export default function Page() {
+          6 |   return <DynamicClient />
+
+        \`ssr: false\` is not allowed with \`next/dynamic\` in Server Components. Please move it into a client component."
+      `)
+    } else {
+      expect(redbox.source).toMatchInlineSnapshot(`
+        "./app/page.js
+        Error:   x \`ssr: false\` is not allowed with \`next/dynamic\` in Server Components. Please move it into a client component.
+           ,-[3:1]
+         1 | import dynamic from 'next/dynamic'
+         2 | 
+         3 | const DynamicClient = dynamic(() => import('./client'), { ssr: false })
+           :                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+         4 | 
+         5 | export default function Page() {
+         6 |   return <DynamicClient />
+           \`----"
+      `)
+    }
+  })
+})

test/e2e/app-dir/dynamic/app/dynamic-mixed-ssr-false/ssr-false-module/index.js

@@ -1,11 +1,11 @@
 import Client from './ssr-false-client'
-import Server from './ssr-false-server'
+// import Server from './ssr-false-server'
 
 export default function Comp() {
   return (
     <>
       <Client />
-      <Server />
+      {/* <Server /> */}
     </>
   )
 }

test/e2e/app-dir/dynamic/app/dynamic/async-client/page.js

@@ -1,3 +1,5 @@
+'use client'
+
 import dynamic from 'next/dynamic'
 
 const Client1 = dynamic(() => import('./client'))

test/e2e/app-dir/dynamic/app/dynamic/dynamic-imports/dynamic-server.js

@@ -3,12 +3,12 @@ import dynamic from 'next/dynamic'
 export const NextDynamicServerComponent = dynamic(
   () => import('../text-dynamic-server')
 )
-export const NextDynamicNoSSRServerComponent = dynamic(
-  () => import('../text-dynamic-no-ssr-server'),
-  {
-    ssr: false,
-  }
-)
+// export const NextDynamicNoSSRServerComponent = dynamic(
+//   () => import('../text-dynamic-no-ssr-server'),
+//   {
+//     ssr: false,
+//   }
+// )
 export const NextDynamicServerImportClientComponent = dynamic(
   () => import('../text-dynamic-server-import-client')
 )

test/e2e/app-dir/dynamic/app/dynamic/page.js

@@ -3,7 +3,7 @@ import { NextDynamicClientComponent } from './dynamic-imports/dynamic-client'
 import {
   NextDynamicServerComponent,
   NextDynamicServerImportClientComponent,
-  NextDynamicNoSSRServerComponent,
+  // NextDynamicNoSSRServerComponent,
 } from './dynamic-imports/dynamic-server'
 
 export default function page() {
@@ -13,7 +13,7 @@ export default function page() {
       <NextDynamicServerComponent />
       <NextDynamicClientComponent />
       <NextDynamicServerImportClientComponent />
-      <NextDynamicNoSSRServerComponent />
+      {/* <NextDynamicNoSSRServerComponent /> */}
     </div>
   )
 }

test/e2e/app-dir/dynamic/app/dynamic/text-dynamic-no-ssr-server.js

@@ -1,12 +1,12 @@
-import TextClient from './text-client'
+// import TextClient from './text-client'
 
-export default function Dynamic() {
-  return (
-    <>
-      <p id="css-text-dynamic-no-ssr-server">
-        next-dynamic dynamic no ssr on server
-      </p>
-      <TextClient />
-    </>
-  )
-}
+// export default function Dynamic() {
+//   return (
+//     <>
+//       <p id="css-text-dynamic-no-ssr-server">
+//         next-dynamic dynamic no ssr on server
+//       </p>
+//       <TextClient />
+//     </>
+//   )
+// }

test/e2e/app-dir/dynamic/dynamic.test.ts

@@ -33,19 +33,10 @@ describe('app dir - next/dynamic', () => {
     expect(serverContent).toContain('next-dynamic dynamic on client')
     expect(serverContent).toContain('next-dynamic server import client')
     expect(serverContent).not.toContain('next-dynamic dynamic no ssr on client')
-
-    expect(serverContent).not.toContain('next-dynamic dynamic no ssr on server')
-
-    // client component under server component with ssr: false will not be rendered either in flight or SSR
-    expect($.html()).not.toContain('client component under sever no ssr')
   })
 
   it('should handle next/dynamic in hydration correctly', async () => {
-    const selector = 'body div'
     const browser = await next.browser('/dynamic')
-    const clientContent = await browser.elementByCss(selector).text()
-    expect(clientContent).toContain('next-dynamic dynamic no ssr on server')
-    expect(clientContent).toContain('client component under sever no ssr')
     await browser.waitForElementByCss('#css-text-dynamic-no-ssr-client')
 
     expect(
@@ -91,17 +82,11 @@ describe('app dir - next/dynamic', () => {
     it('should not render client component imported through ssr: false in client components in edge runtime', async () => {
       // noSSR should not show up in html
       const $ = await next.render$('/dynamic-mixed-ssr-false/client-edge')
-      expect($('#server-false-server-module')).not.toContain(
-        'ssr-false-server-module-text'
-      )
       expect($('#server-false-client-module')).not.toContain(
         'ssr-false-client-module-text'
       )
       // noSSR should not show up in browser
       const browser = await next.browser('/dynamic-mixed-ssr-false/client-edge')
-      expect(
-        await browser.elementByCss('#ssr-false-server-module').text()
-      ).toBe('ssr-false-server-module-text')
       expect(
         await browser.elementByCss('#ssr-false-client-module').text()
       ).toBe('ssr-false-client-module-text')
@@ -119,17 +104,11 @@ describe('app dir - next/dynamic', () => {
     it('should not render client component imported through ssr: false in client components', async () => {
       // noSSR should not show up in html
       const $ = await next.render$('/dynamic-mixed-ssr-false/client')
-      expect($('#client-false-server-module')).not.toContain(
-        'ssr-false-server-module-text'
-      )
       expect($('#client-false-client-module')).not.toContain(
         'ssr-false-client-module-text'
       )
       // noSSR should not show up in browser
       const browser = await next.browser('/dynamic-mixed-ssr-false/client')
-      expect(
-        await browser.elementByCss('#ssr-false-server-module').text()
-      ).toBe('ssr-false-server-module-text')
       expect(
         await browser.elementByCss('#ssr-false-client-module').text()
       ).toBe('ssr-false-client-module-text')
@@ -139,7 +118,6 @@ describe('app dir - next/dynamic', () => {
         const pageServerChunk = await next.readFile(
           '.next/server/app/dynamic-mixed-ssr-false/client/page.js'
         )
-        expect(pageServerChunk).not.toContain('ssr-false-server-module-text')
         expect(pageServerChunk).not.toContain('ssr-false-client-module-text')
       }
     })

test/e2e/app-dir/missing-suspense-with-csr-bailout/app/dynamic/page.js

@@ -1,3 +1,5 @@
+'use client'
+
 import dynamic from 'next/dynamic'
 
 const Dynamic = dynamic(() => import('./dynamic'), {

test/integration/next-image-new/app-dir/app/dynamic-static-img/async-image.js

@@ -0,0 +1,10 @@
+'use client'
+
+import dynamic from 'next/dynamic'
+
+export const DynamicStaticImg = dynamic(
+  () => import('../../components/static-img'),
+  {
+    ssr: false,
+  }
+)