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

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

Merged
merged 8 commits into from
Sep 25, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -76,6 +76,7 @@ enum RSCErrorKind {
NextRscErrConflictMetadataExport(Span),
NextRscErrInvalidApi((String, Span)),
NextRscErrDeprecatedApi((String, String, Span)),
NextSsrDynamicFalseNotAllowed(Span),
}

impl<C: Comments> VisitMut for ReactServerComponents<C> {
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(),
}
}

Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import dynamic from 'next/dynamic'

export default function () {
return dynamic(() => import('client-only'), { ssr: false })
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import dynamic from 'next/dynamic';
export default function() {
return dynamic(()=>import('client-only'), {
ssr: false
});
}
Original file line number Diff line number Diff line change
@@ -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 | }
`----
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client'

export default function Client() {
return 'client'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Root({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import dynamic from 'next/dynamic'

const DynamicClient = dynamic(() => import('./client'), { ssr: false })

export default function Page() {
return <DynamicClient />
}
Original file line number Diff line number Diff line change
@@ -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 />
\`----"
`)
}
})
})
Original file line number Diff line number Diff line change
@@ -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 /> */}
</>
)
}
2 changes: 2 additions & 0 deletions test/e2e/app-dir/dynamic/app/dynamic/async-client/page.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client'

import dynamic from 'next/dynamic'

const Client1 = dynamic(() => import('./client'))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
)
4 changes: 2 additions & 2 deletions test/e2e/app-dir/dynamic/app/dynamic/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -13,7 +13,7 @@ export default function page() {
<NextDynamicServerComponent />
<NextDynamicClientComponent />
<NextDynamicServerImportClientComponent />
<NextDynamicNoSSRServerComponent />
{/* <NextDynamicNoSSRServerComponent /> */}
</div>
)
}
22 changes: 11 additions & 11 deletions test/e2e/app-dir/dynamic/app/dynamic/text-dynamic-no-ssr-server.js
Original file line number Diff line number Diff line change
@@ -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 />
// </>
// )
// }
22 changes: 0 additions & 22 deletions test/e2e/app-dir/dynamic/dynamic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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')
}
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client'

import dynamic from 'next/dynamic'

const Dynamic = dynamic(() => import('./dynamic'), {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use client'

import dynamic from 'next/dynamic'

export const DynamicStaticImg = dynamic(
() => import('../../components/static-img'),
{
ssr: false,
}
)
Loading
Loading