Skip to content

Commit

Permalink
Improve RSC compiler error in external module (#46953)
Browse files Browse the repository at this point in the history
When the RSC compiler error was caused by an external package, make the
error message display which import caused the error. Also don't show
node_module files in the import trace.

Continuation of #45484

Before

![image](https://user-images.githubusercontent.com/25056922/224032476-6811a1d5-d690-48be-9602-781f459edc70.png)

After

![image](https://user-images.githubusercontent.com/25056922/224032177-2d0b2977-098f-46bd-8e30-9e6bc21b9153.png)

Updates the format of the files, from `app/page.js` to `./app/page.js`
to align it with other import traces.

![image](https://user-images.githubusercontent.com/25056922/224030420-1d3ff0ba-5747-4ed3-8b0b-9c4deace54ea.png)


Closes NEXT-523
  • Loading branch information
hanneslund authored Mar 9, 2023
1 parent 09d21d6 commit 9a41ba9
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 73 deletions.
Original file line number Diff line number Diff line change
@@ -1,18 +1,59 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'
import loaderUtils from 'next/dist/compiled/loader-utils3'
import { relative } from 'path'

export function formatModule(compiler: webpack.Compiler, module: any) {
return relative(compiler.context, module.resource).replace(/\?.+$/, '')
function formatModule(compiler: webpack.Compiler, module: any) {
const relativePath = relative(compiler.context, module.resource).replace(
/\?.+$/,
''
)
return loaderUtils.isUrlRequest(relativePath)
? loaderUtils.urlToRequest(relativePath)
: relativePath
}

export function getImportTraceForOverlay(
export function formatModuleTrace(
compiler: webpack.Compiler,
moduleTrace: any[]
) {
return moduleTrace
.map((m) => (m.resource ? ' ' + formatModule(compiler, m) : ''))
.filter(Boolean)
.join('\n')
let importTrace: string[] = []
let firstExternalModule: any
for (let i = moduleTrace.length - 1; i >= 0; i--) {
const mod = moduleTrace[i]
if (!mod.resource) continue

if (!mod.resource.includes('node_modules/')) {
importTrace.unshift(formatModule(compiler, mod))
} else {
firstExternalModule = mod
break
}
}

let invalidImportMessage = ''
if (firstExternalModule) {
const firstExternalPackageName =
firstExternalModule.resourceResolveData?.descriptionFileData?.name

if (firstExternalPackageName === 'styled-jsx') {
invalidImportMessage += `\n\nThe error was caused by using 'styled-jsx' in '${importTrace[0]}'. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.`
} else {
let formattedExternalFile =
firstExternalModule.resource.split('node_modules')
formattedExternalFile =
formattedExternalFile[formattedExternalFile.length - 1]

invalidImportMessage += `\n\nThe error was caused by importing '${formattedExternalFile.slice(
1
)}' in '${importTrace[0]}'.`
}
}

return {
lastInternalFileName: importTrace[0],
invalidImportMessage,
formattedModuleTrace: `${importTrace.map((mod) => ' ' + mod).join('\n')}`,
}
}

export function getModuleTrace(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'
import { formatModule, getModuleTrace } from './getModuleTrace'
import { formatModuleTrace, getModuleTrace } from './getModuleTrace'
import { SimpleWebpackError } from './simpleWebpackError'

export function getNextInvalidImportError(
Expand All @@ -18,49 +18,15 @@ export function getNextInvalidImportError(
}

const { moduleTrace } = getModuleTrace(module, compilation, compiler)

let importTrace: string[] = []
let firstExternalModule: any
for (let i = moduleTrace.length - 1; i >= 0; i--) {
const mod = moduleTrace[i]
if (!mod.resource) continue

if (!mod.resource.includes('node_modules/')) {
importTrace.unshift(formatModule(compiler, mod))
} else {
firstExternalModule = mod
break
}
}

let invalidImportMessage = ''
if (firstExternalModule) {
const firstExternalPackageName =
firstExternalModule.resourceResolveData?.descriptionFileData?.name

if (firstExternalPackageName === 'styled-jsx') {
invalidImportMessage += `\n\nThe error was caused by using 'styled-jsx' in '${importTrace[0]}'. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.`
} else {
let formattedExternalFile =
firstExternalModule.resource.split('node_modules')
formattedExternalFile =
formattedExternalFile[formattedExternalFile.length - 1]

invalidImportMessage += `\n\nThe error was caused by importing '${formattedExternalFile.slice(
1
)}' in '${importTrace[0]}'.`
}
}
const { formattedModuleTrace, lastInternalFileName, invalidImportMessage } =
formatModuleTrace(compiler, moduleTrace)

return new SimpleWebpackError(
importTrace[0],
lastInternalFileName,
err.message +
invalidImportMessage +
(importTrace.length > 0
? `\n\nImport trace for requested module:\n${importTrace
.map((mod) => ' ' + mod)
.join('\n')}`
: '')
'\n\nImport trace for requested module:\n' +
formattedModuleTrace
)
} catch {
return false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'

import { getModuleTrace, getImportTraceForOverlay } from './getModuleTrace'
import { getModuleTrace, formatModuleTrace } from './getModuleTrace'
import { SimpleWebpackError } from './simpleWebpackError'

function formatRSCErrorMessage(
Expand Down Expand Up @@ -141,12 +141,16 @@ export function getRscError(
fileName
)

const { formattedModuleTrace, lastInternalFileName, invalidImportMessage } =
formatModuleTrace(compiler, moduleTrace)

const error = new SimpleWebpackError(
fileName,
lastInternalFileName,
'ReactServerComponentsError:\n' +
formattedError[0] +
invalidImportMessage +
formattedError[1] +
getImportTraceForOverlay(compiler, moduleTrace)
formattedModuleTrace
)

// Delete the stack because it's created here.
Expand Down
30 changes: 15 additions & 15 deletions test/development/acceptance-app/invalid-imports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ createNextDescribe(

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"app/comp2.js
"./app/comp2.js
'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.
The error was caused by using 'styled-jsx' in 'app/comp2.js'. It only works in a Client Component but none of its parents are marked with \\"use client\\", so they're Server Components by default.
The error was caused by using 'styled-jsx' in './app/comp2.js'. It only works in a Client Component but none of its parents are marked with \\"use client\\", so they're Server Components by default.
Import trace for requested module:
app/comp2.js
app/comp1.js
app/page.js"
./app/comp2.js
./app/comp1.js
./app/page.js"
`)

await cleanup()
Expand Down Expand Up @@ -142,15 +142,15 @@ createNextDescribe(

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"app/comp2.js
"./app/comp2.js
'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.
The error was caused by importing 'client-only-package/index.js' in 'app/comp2.js'.
The error was caused by importing 'client-only-package/index.js' in './app/comp2.js'.
Import trace for requested module:
app/comp2.js
app/comp1.js
app/page.js"
./app/comp2.js
./app/comp1.js
./app/page.js"
`)

await cleanup()
Expand Down Expand Up @@ -215,15 +215,15 @@ createNextDescribe(

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"app/comp2.js
"./app/comp2.js
'server-only' cannot be imported from a Client Component module. It should only be used from a Server Component.
The error was caused by importing 'server-only-package/index.js' in 'app/comp2.js'.
The error was caused by importing 'server-only-package/index.js' in './app/comp2.js'.
Import trace for requested module:
app/comp2.js
app/comp1.js
app/page.js"
./app/comp2.js
./app/comp1.js
./app/page.js"
`)

await cleanup()
Expand Down
61 changes: 57 additions & 4 deletions test/development/acceptance-app/rsc-build-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ createNextDescribe(
\`----
Import path:
app/server-with-errors/error-file/error.js"
./app/server-with-errors/error-file/error.js"
`)

await cleanup()
Expand Down Expand Up @@ -314,7 +314,7 @@ createNextDescribe(
\`----
Import path:
app/server-with-errors/error-file/error.js"
./app/server-with-errors/error-file/error.js"
`)

await cleanup()
Expand Down Expand Up @@ -363,8 +363,8 @@ createNextDescribe(
\`----
Maybe one of these should be marked as a client entry with \\"use client\\":
app/editor-links/component.js
app/editor-links/page.js"
./app/editor-links/component.js
./app/editor-links/page.js"
`)

await browser.waitForElementByCss('[data-with-open-in-editor-link]')
Expand Down Expand Up @@ -413,5 +413,58 @@ createNextDescribe(

await cleanup()
})

it('should show which import caused an error in node_modules', async () => {
const { session, cleanup } = await sandbox(
next,
new Map([
[
'node_modules/client-package/module2.js',
"import { useState } from 'react'",
],
['node_modules/client-package/module1.js', "import './module2.js'"],
['node_modules/client-package/index.js', "import './module1.js'"],
[
'node_modules/client-package/package.json',
`
{
"name": "client-package",
"version": "0.0.1"
}
`,
],
['app/Component.js', "import 'client-package'"],
[
'app/page.js',
`
import './Component.js'
export default function Page() {
return <p>Hello world</p>
}`,
],
])
)

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./app/Component.js
ReactServerComponentsError:
You're importing a component that needs useState. It only works in a Client Component but none of its parents are marked with \\"use client\\", so they're Server Components by default.
,----
1 | import { useState } from 'react'
: ^^^^^^^^
\`----
The error was caused by importing 'client-package/index.js' in './app/Component.js'.
Maybe one of these should be marked as a client entry with \\"use client\\":
./app/Component.js
./app/page.js"
`)

await cleanup()
})
}
)
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ createNextDescribe(
\`----
Import trace for requested module:
components/Comp.js
pages/index.js"
./components/Comp.js
./pages/index.js"
`)

await cleanup()
Expand Down Expand Up @@ -106,8 +106,8 @@ createNextDescribe(
\`----
Import trace for requested module:
components/Comp.js
pages/index.js"
./components/Comp.js
./pages/index.js"
`)

await cleanup()
Expand Down

0 comments on commit 9a41ba9

Please sign in to comment.