Skip to content

Commit

Permalink
Do not strip loader arg in dynamic for server components (#42426)
Browse files Browse the repository at this point in the history
When `ssr: false` option is presented, we stripped the loader option for
`next/dynamic` call. But for react server components (both server and
client ones) we're using a `React.lazy` based implementation so we
shouldn't do it for them.


## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`
  • Loading branch information
huozhi authored Nov 3, 2022
1 parent 152f51c commit 21b6654
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/next-swc/crates/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
),
Expand Down
18 changes: 15 additions & 3 deletions packages/next-swc/crates/core/src/next_dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ use swc_core::{
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![],
Expand All @@ -35,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>,
Expand Down Expand Up @@ -279,9 +282,18 @@ impl Fold for NextDynamicPatcher {
props.extend(options_props.iter().cloned());
}
}
// Don't need to strip the `loader` argument if suspense is true
// See https://github.com/vercel/next.js/issues/36636 for background
if has_ssr_false && !has_suspense && self.is_server {

// Don't strip the `loader` argument if suspense is true
// See https://github.com/vercel/next.js/issues/36636 for background.

// Also don't strip the `loader` argument for server components (both
// server/client layers), since they're aliased to a
// React.lazy implementation.
if has_ssr_false
&& !has_suspense
&& self.is_server
&& !self.is_server_components
{
expr.args[0] = Lit::Null(Null { span: DUMMY_SP }).as_arg();
}

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 @@ -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()),
)
Expand Down
3 changes: 3 additions & 0 deletions packages/next-swc/crates/core/tests/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
)
Expand All @@ -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")),
Expand All @@ -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()),
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import dynamic from 'next/dynamic'

const Dynamic = dynamic(() => import('../text-dynamic-server'))
const Dynamic = dynamic(() => import('../text-dynamic-server'), {
ssr: false,
})

export function NextDynamicServerComponent() {
return <Dynamic />
Expand Down

0 comments on commit 21b6654

Please sign in to comment.