Skip to content

Commit

Permalink
SWC: Add error checks and tests to next-dynamic (#31683)
Browse files Browse the repository at this point in the history
fixes #31520

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
  • Loading branch information
hanneslund authored Nov 26, 2021
1 parent 77150da commit 466687e
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 1 deletion.
31 changes: 31 additions & 0 deletions errors/invalid-dynamic-options-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Invalid options type in a `next/dynamic` call

#### Why This Error Occurred

You have an invalid options type in a `next/dynamic` call. The options must be an object literal.

#### Possible Ways to Fix It

**Before**

```jsx
import dynamic from 'next/dynamic'

const options = { loading: () => <p>...</p>, ssr: false }
const DynamicComponent = dynamic(() => import('../components/hello'), options)
```

**After**

```jsx
import dynamic from 'next/dynamic'

const DynamicComponent = dynamic(() => import('../components/hello'), {
loading: () => <p>...</p>,
ssr: false,
})
```

### Useful Links

- [Dynamic Import](https://nextjs.org/docs/advanced-features/dynamic-import)
4 changes: 4 additions & 0 deletions errors/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@
{
"title": "experimental-jest-transformer",
"path": "/errors/experimental-jest-transformer.md"
},
{
"title": "invalid-dynamic-options-type",
"path": "/errors/invalid-dynamic-options-type.md"
}
]
}
Expand Down
17 changes: 17 additions & 0 deletions packages/next-swc/crates/core/src/next_dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,28 @@ impl Fold for NextDynamicPatcher {
});
return expr;
}
if expr.args.len() == 2 {
match &*expr.args[1].expr {
Expr::Object(_) => {},
_ => {
HANDLER.with(|handler| {
handler
.struct_span_err(
identifier.span,
"next/dynamic options must be an object literal.\nRead more: https://nextjs.org/docs/messages/invalid-dynamic-options-type",
)
.emit();
});
return expr;
}
}
}

self.is_next_dynamic_first_arg = true;
expr.args[0].expr = expr.args[0].expr.clone().fold_with(self);
self.is_next_dynamic_first_arg = false;


if let None = self.dynamically_imported_specifier {
return expr;
}
Expand Down
21 changes: 20 additions & 1 deletion packages/next-swc/crates/core/tests/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use next_swc::disallow_re_export_all_in_page::disallow_re_export_all_in_page;
use next_swc::{
disallow_re_export_all_in_page::disallow_re_export_all_in_page, next_dynamic::next_dynamic,
};
use std::path::PathBuf;
use swc_common::FileName;
use swc_ecma_transforms_testing::test_fixture_allowing_error;
use swc_ecmascript::parser::{EsConfig, Syntax};
use testing::fixture;
Expand All @@ -22,3 +25,19 @@ fn re_export_all_in_page(input: PathBuf) {
&output,
);
}

#[fixture("tests/errors/next-dynamic/**/input.js")]
fn next_dynamic_errors(input: PathBuf) {
let output = input.parent().unwrap().join("output.js");
test_fixture_allowing_error(
syntax(),
&|_tr| {
next_dynamic(
FileName::Real(PathBuf::from("/some-project/src/some-file.js")),
Some("/some-project/src".into()),
)
},
&input,
&output,
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import dynamic from 'next/dynamic'

const DynamicComponent = dynamic()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import dynamic from 'next/dynamic'

const DynamicComponent = dynamic()
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error: next/dynamic requires at least one argument
--> input.js:3:26
|
3 | const DynamicComponent = dynamic()
| ^^^^^^^

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import dynamic from 'next/dynamic'

const options = { loading: () => <p>...</p>, ssr: false }
const DynamicComponentWithCustomLoading = dynamic(
() => import('../components/hello'),
options
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import dynamic from 'next/dynamic';

const options = { loading: () => <p>...</p>, ssr: false };
const DynamicComponentWithCustomLoading = dynamic(
() => import('../components/hello'),
options
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error: next/dynamic options must be an object literal.
Read more: https://nextjs.org/docs/messages/invalid-dynamic-options-type
--> input.js:4:43
|
4 | const DynamicComponentWithCustomLoading = dynamic(
| ^^^^^^^

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import dynamic from 'next/dynamic'

const DynamicComponentWithCustomLoading = dynamic(
() => import('../components/hello'),
{ loading: () => <p>...</p> },
"3rd"
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import dynamic from 'next/dynamic'

const DynamicComponentWithCustomLoading = dynamic(
() => import('../components/hello'),
{ loading: () => <p>...</p> },
"3rd"
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error: next/dynamic only accepts 2 arguments
--> input.js:3:43
|
3 | const DynamicComponentWithCustomLoading = dynamic(
| ^^^^^^^

0 comments on commit 466687e

Please sign in to comment.