Skip to content

Commit

Permalink
Refine error messages (#40661)
Browse files Browse the repository at this point in the history
As per @sebmarkbage's suggestion, this PR refines the error message to
be more readable and friendly:

```
error - ./components/qux.js

You're importing a component that needs useEffect. It only works in a Client Component but none of its parents are marked with "client", so they're Server Components by default.

   ,----
 5 | import { useEffect } from 'react'
   :          ^^^^^^^^^
   `----

Maybe one of these should be marked as a "client" entry:

./components/qux.js
./components/baz.js
./components/bar.js
./app/dashboard/index/page.js
```

It's more ideal to put error codes inside the SWC transform and format
it in the Next.js logging layer. A future improvement is to tweak the
message a bit more for these specific cases:

- [ ] The error already happens inside an entry point (page or layout),
so itself should have "client" added (or removed), not it parents.
- [ ] When importing `"server-only"` inside a client component, we can
directly hint to the user which module in the import chain is marked
with `"client"`.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a 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 a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
shuding committed Sep 19, 2022
1 parent 6df8770 commit 44afc37
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 67 deletions.
26 changes: 4 additions & 22 deletions packages/next-swc/crates/core/src/react_server_components.rs
Expand Up @@ -227,11 +227,7 @@ impl<C: Comments> ReactServerComponents<C> {
handler
.struct_span_err(
import.source.1,
format!(
"Disallowed import of `{}` in the Server Components compilation.",
source
)
.as_str(),
format!("NEXT_RSC_ERR_SERVER_IMPORT: {}", source).as_str(),
)
.emit()
})
Expand All @@ -243,12 +239,7 @@ impl<C: Comments> ReactServerComponents<C> {
handler
.struct_span_err(
specifier.1,
format!(
"Disallowed React API `{}` in the Server Components \
compilation.",
&specifier.0
)
.as_str(),
format!("NEXT_RSC_ERR_REACT_API: {}", &specifier.0).as_str(),
)
.emit()
})
Expand All @@ -262,12 +253,7 @@ impl<C: Comments> ReactServerComponents<C> {
handler
.struct_span_err(
specifier.1,
format!(
"Disallowed ReactDOM API `{}` in the Server Components \
compilation.",
&specifier.0
)
.as_str(),
format!("NEXT_RSC_ERR_REACT_API: {}", &specifier.0).as_str(),
)
.emit()
})
Expand All @@ -285,11 +271,7 @@ impl<C: Comments> ReactServerComponents<C> {
handler
.struct_span_err(
import.source.1,
format!(
"Disallowed import of `{}` in the Client Components compilation.",
source
)
.as_str(),
format!("NEXT_RSC_ERR_CLIENT_IMPORT: {}", source).as_str(),
)
.emit()
})
Expand Down
@@ -1,5 +1,5 @@

x Disallowed import of `server-only` in the Client Components compilation.
x NEXT_RSC_ERR_CLIENT_IMPORT: server-only
,-[input.js:9:1]
9 | import "server-only"
: ^^^^^^^^^^^^^^^^^^^^
Expand Down
@@ -1,5 +1,5 @@

x Disallowed import of `client-only` in the Server Components compilation.
x NEXT_RSC_ERR_SERVER_IMPORT: client-only
,-[input.js:9:1]
9 | import "client-only"
: ^^^^^^^^^^^^^^^^^^^^
Expand Down
@@ -1,77 +1,77 @@

x Disallowed React API `useState` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useState
,-[input.js:1:1]
1 | import { useState } from 'react'
: ^^^^^^^^
`----

x Disallowed React API `createContext` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: createContext
,-[input.js:3:1]
3 | import { createContext } from 'react'
: ^^^^^^^^^^^^^
`----

x Disallowed React API `useEffect` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useEffect
,-[input.js:5:1]
5 | import { useEffect, useImperativeHandle } from 'react'
: ^^^^^^^^^
`----

x Disallowed React API `useImperativeHandle` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useImperativeHandle
,-[input.js:5:1]
5 | import { useEffect, useImperativeHandle } from 'react'
: ^^^^^^^^^^^^^^^^^^^
`----

x Disallowed React API `Component` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: Component
,-[input.js:8:5]
8 | Component,
: ^^^^^^^^^
`----

x Disallowed React API `createFactory` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: createFactory
,-[input.js:9:5]
9 | createFactory,
: ^^^^^^^^^^^^^
`----

x Disallowed React API `PureComponent` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: PureComponent
,-[input.js:10:5]
10 | PureComponent,
: ^^^^^^^^^^^^^
`----

x Disallowed React API `useDeferredValue` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useDeferredValue
,-[input.js:11:3]
11 | useDeferredValue,
: ^^^^^^^^^^^^^^^^
`----

x Disallowed React API `useInsertionEffect` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useInsertionEffect
,-[input.js:12:5]
12 | useInsertionEffect,
: ^^^^^^^^^^^^^^^^^^
`----

x Disallowed React API `useLayoutEffect` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useLayoutEffect
,-[input.js:13:5]
13 | useLayoutEffect,
: ^^^^^^^^^^^^^^^
`----

x Disallowed React API `useReducer` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useReducer
,-[input.js:14:5]
14 | useReducer,
: ^^^^^^^^^^
`----

x Disallowed React API `useRef` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useRef
,-[input.js:15:5]
15 | useRef,
: ^^^^^^
`----

x Disallowed React API `useSyncExternalStore` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: useSyncExternalStore
,-[input.js:16:5]
16 | useSyncExternalStore
: ^^^^^^^^^^^^^^^^^^^^
Expand Down
@@ -1,17 +1,17 @@

x Disallowed ReactDOM API `findDOMNode` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: findDOMNode
,-[input.js:2:5]
2 | findDOMNode,
: ^^^^^^^^^^^
`----

x Disallowed ReactDOM API `flushSync` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: flushSync
,-[input.js:3:3]
3 | flushSync,
: ^^^^^^^^^
`----

x Disallowed ReactDOM API `unstable_batchedUpdates` in the Server Components compilation.
x NEXT_RSC_ERR_REACT_API: unstable_batchedUpdates
,-[input.js:4:3]
4 | unstable_batchedUpdates,
: ^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
@@ -1,11 +1,11 @@

x Disallowed import of `react-dom/server` in the Server Components compilation.
x NEXT_RSC_ERR_SERVER_IMPORT: react-dom/server
,-[input.js:9:1]
9 | import "react-dom/server"
: ^^^^^^^^^^^^^^^^^^^^^^^^^
`----

x Disallowed import of `react-dom/client` in the Server Components compilation.
x NEXT_RSC_ERR_SERVER_IMPORT: react-dom/client
,-[input.js:11:1]
11 | import "react-dom/client"
: ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
32 changes: 15 additions & 17 deletions packages/next/build/webpack-config.ts
Expand Up @@ -1474,24 +1474,22 @@ export default async function getBaseWebpackConfig(
} as any,
]
: []),
...(hasServerComponents
? isNodeServer || isEdgeServer
? [
// RSC server compilation loaders
{
test: codeCondition.test,
include: [
dir,
// To let the internal client components passing through flight loader
/next[\\/]dist/,
],
issuerLayer: WEBPACK_LAYERS.server,
use: {
loader: 'next-flight-loader',
},
...(hasServerComponents && (isNodeServer || isEdgeServer)
? [
// RSC server compilation loaders
{
test: codeCondition.test,
include: [
dir,
// To let the internal client components passing through flight loader
/next[\\/]dist/,
],
issuerLayer: WEBPACK_LAYERS.server,
use: {
loader: 'next-flight-loader',
},
]
: []
},
]
: []),
...(hasServerComponents && isEdgeServer
? [
Expand Down
42 changes: 34 additions & 8 deletions packages/next/client/dev/error-overlay/format-webpack-messages.js
Expand Up @@ -37,7 +37,7 @@ function isLikelyASyntaxError(message) {
let hadMissingSassError = false

// Cleans up webpack error messages.
function formatMessage(message, verbose) {
function formatMessage(message, verbose, importTraceNote) {
// TODO: Replace this once webpack 5 is stable
if (typeof message === 'object' && message.message) {
const filteredModuleTrace =
Expand All @@ -61,7 +61,7 @@ function formatMessage(message, verbose) {
body +
(message.details && verbose ? '\n' + message.details : '') +
(filteredModuleTrace && filteredModuleTrace.length && verbose
? '\n\nImport trace for requested module:' +
? (importTraceNote || '\n\nImport trace for requested module:') +
filteredModuleTrace.map((trace) => `\n${trace.moduleName}`).join('')
: '') +
(message.stack && verbose ? '\n' + message.stack : '')
Expand Down Expand Up @@ -171,18 +171,44 @@ function formatMessage(message, verbose) {

function formatWebpackMessages(json, verbose) {
const formattedErrors = json.errors.map(function (message) {
let importTraceNote

// TODO: Shall we use invisible characters in the original error
// message as meta information?
if (
message &&
message.message &&
/ (Client|Server) Components compilation\./.test(message.message)
) {
if (message && message.message && /NEXT_RSC_ERR_/.test(message.message)) {
// Comes from the "React Server Components" transform in SWC, always
// attach the module trace.
const NEXT_RSC_ERR_REACT_API = /.+NEXT_RSC_ERR_REACT_API: (.*?)\n/s
const NEXT_RSC_ERR_SERVER_IMPORT =
/.+NEXT_RSC_ERR_SERVER_IMPORT: (.*?)\n/s
const NEXT_RSC_ERR_CLIENT_IMPORT =
/.+NEXT_RSC_ERR_CLIENT_IMPORT: (.*?)\n/s

if (NEXT_RSC_ERR_REACT_API.test(message.message)) {
message.message = message.message.replace(
NEXT_RSC_ERR_REACT_API,
`\n\nYou're importing a component that needs $1. It only works in a Client Component but none of its parents are marked with "client", so they're Server Components by default.\n\n`
)
importTraceNote =
'\n\nMaybe one of these should be marked as a "client" entry:\n'
} else if (NEXT_RSC_ERR_SERVER_IMPORT.test(message.message)) {
message.message = message.message.replace(
NEXT_RSC_ERR_SERVER_IMPORT,
`\n\nYou're importing a component that imports $1. It only works in a Client Component but none of its parents are marked with "client", so they're Server Components by default.\n\n`
)
importTraceNote =
'\n\nMaybe one of these should be marked as a "client" entry:\n'
} else if (NEXT_RSC_ERR_CLIENT_IMPORT.test(message.message)) {
message.message = message.message.replace(
NEXT_RSC_ERR_CLIENT_IMPORT,
`\n\nYou're importing a component that needs $1. That only works in a Server Component but one of its parents is marked with "client", so it's a Client Component.\n\n`
)
importTraceNote = '\n\nOne of these is marked as a "client" entry:\n'
}

verbose = true
}
return formatMessage(message, verbose)
return formatMessage(message, verbose, importTraceNote)
})
const formattedWarnings = json.warnings.map(function (message) {
return formatMessage(message, verbose)
Expand Down

0 comments on commit 44afc37

Please sign in to comment.