Skip to content

Commit

Permalink
Improve client-only imported in external package error (vercel#45484)
Browse files Browse the repository at this point in the history
Currently if an external package imports `'client-only'` it's difficult to figure out why the error occured. This change adds an explanation of which import caused the error.

The errors in the images are from using `<style jsx>` in a Server Component.

Before, runtime error on the server
![image](https://user-images.githubusercontent.com/25056922/216080617-9eebf99e-2899-4d7e-80b6-ef7ded0831ce.png)

After, webpack error with added stack trace
![image](https://user-images.githubusercontent.com/25056922/216303311-05c2f3ca-2857-4fe7-b55c-caaccf5f46bb.png)


Fixes NEXT-409

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/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`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) 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`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && 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
hanneslund authored and jankaifer committed Feb 14, 2023
1 parent 2e4ef53 commit 39e1670
Show file tree
Hide file tree
Showing 10 changed files with 236 additions and 39 deletions.
10 changes: 10 additions & 0 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1651,6 +1651,7 @@ export default async function getBaseWebpackConfig(
'next-middleware-wasm-loader',
'next-app-loader',
'next-font-loader',
'next-invalid-import-error-loader',
].reduce((alias, loader) => {
// using multiple aliases to replace `resolveLoader.modules`
alias[loader] = path.join(__dirname, 'webpack', 'loaders', loader)
Expand Down Expand Up @@ -1991,6 +1992,15 @@ export default async function getBaseWebpackConfig(
},
]
: []),
{
test: /node_modules\/client-only\/error.js/,
loader: 'next-invalid-import-error-loader',
issuerLayer: WEBPACK_LAYERS.server,
options: {
message:
"'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.",
},
},
].filter(Boolean),
},
plugins: [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default function nextInvalidImportErrorLoader(this: any) {
const { message } = this.getOptions()
throw new Error(message)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'
import { relative } from 'path'

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

export function getImportTraceForOverlay(
compiler: webpack.Compiler,
moduleTrace: any[]
) {
return moduleTrace
.map((m) => (m.resource ? ' ' + formatModule(compiler, m) : ''))
.filter(Boolean)
.join('\n')
}

export function getModuleTrace(
module: any,
compilation: webpack.Compilation,
compiler: webpack.Compiler
) {
// Get the module trace:
// https://cs.github.com/webpack/webpack/blob/9fcaa243573005d6fdece9a3f8d89a0e8b399613/lib/stats/DefaultStatsFactoryPlugin.js#L414
const visitedModules = new Set()
const moduleTrace = []

let current = module
let isPagesDir = false
while (current) {
if (visitedModules.has(current)) break
if (/[\\/]pages/.test(current.resource.replace(compiler.context, ''))) {
isPagesDir = true
}
visitedModules.add(current)
moduleTrace.push(current)
const origin = compilation.moduleGraph.getIssuer(current)
if (!origin) break
current = origin
}

return {
moduleTrace,
isPagesDir,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'
import { formatModule, getModuleTrace } from './getModuleTrace'
import { SimpleWebpackError } from './simpleWebpackError'

export function getNextInvalidImportError(
err: Error,
module: any,
compilation: webpack.Compilation,
compiler: webpack.Compiler
): SimpleWebpackError | false {
try {
if (
!module.loaders.find((loader: any) =>
loader.loader.includes('next-invalid-import-error-loader.js')
)
) {
return false
}

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

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

function formatRSCErrorMessage(
Expand Down Expand Up @@ -127,24 +127,11 @@ export function getRscError(
return false
}

// Get the module trace:
// https://cs.github.com/webpack/webpack/blob/9fcaa243573005d6fdece9a3f8d89a0e8b399613/lib/stats/DefaultStatsFactoryPlugin.js#L414
const visitedModules = new Set()
const moduleTrace = []

let current = module
let isPagesDir = false
while (current) {
if (visitedModules.has(current)) break
if (/[\\/]pages/.test(current.resource.replace(compiler.context, ''))) {
isPagesDir = true
}
visitedModules.add(current)
moduleTrace.push(current)
const origin = compilation.moduleGraph.getIssuer(current)
if (!origin) break
current = origin
}
const { isPagesDir, moduleTrace } = getModuleTrace(
module,
compilation,
compiler
)

const formattedError = formatRSCErrorMessage(
err.message,
Expand All @@ -157,14 +144,7 @@ export function getRscError(
'ReactServerComponentsError:\n' +
formattedError[0] +
formattedError[1] +
moduleTrace
.map((m) =>
m.resource
? ' ' + relative(compiler.context, m.resource).replace(/\?.+$/, '')
: ''
)
.filter(Boolean)
.join('\n')
getImportTraceForOverlay(compiler, moduleTrace)
)

// Delete the stack because it's created here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import isError from '../../../../lib/is-error'
import { getRscError } from './parseRSC'
import { getNextFontError } from './parseNextFontError'
import { getNextAppLoaderError } from './parseNextAppLoaderError'
import { getNextInvalidImportError } from './parseNextInvalidImportError'

function getFileData(
compilation: webpack.Compilation,
Expand Down Expand Up @@ -114,5 +115,15 @@ export async function getModuleBuildError(
return nextAppLoader
}

const invalidImportError = getNextInvalidImportError(
err,
input.module,
compilation,
compiler
)
if (invalidImportError !== false) {
return invalidImportError
}

return false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Comp2 } from './comp2'

export function Comp1() {
return <Comp2 />
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export function Comp2() {
return (
<div>
<style jsx>{`
p {
color: red;
}
`}</style>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
'use client'
import { Comp1 } from './comp1'

export default function Page() {
return (
<div>
<style jsx>{`
p {
color: red;
}
`}</style>
</div>
)
return <Comp1 />
}
80 changes: 77 additions & 3 deletions test/development/acceptance-app/rsc-build-errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createNextDescribe, FileRef } from 'e2e-utils'
import { check } from 'next-test-utils'
import path from 'path'
import { sandbox } from './helpers'

Expand Down Expand Up @@ -67,10 +68,23 @@ createNextDescribe(
'/server-with-errors/styled-jsx'
)

const pageFile = 'app/server-with-errors/styled-jsx/page.js'
const content = await next.readFile(pageFile)
const withoutUseClient = content.replace("'use client'", '')
await session.patch(pageFile, withoutUseClient)

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxDescription()).toInclude(
'This module cannot be imported from a Server Component module. It should only be used from a Client Component.'
)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"app/server-with-errors/styled-jsx/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 'styled-jsx/style.js' in 'app/server-with-errors/styled-jsx/comp2.js'.
Import trace for requested module:
app/server-with-errors/styled-jsx/comp2.js
app/server-with-errors/styled-jsx/comp1.js
app/server-with-errors/styled-jsx/page.js"
`)

await cleanup()
})
Expand Down Expand Up @@ -266,5 +280,65 @@ createNextDescribe(

await cleanup()
})

it('should be possible to open the import trace files in your editor', async () => {
let editorRequestsCount = 0
const { session, browser, cleanup } = await sandbox(
next,
undefined,
'/editor-links',
{
beforePageLoad(page) {
page.route('**/__nextjs_launch-editor**', (route) => {
editorRequestsCount += 1
route.fulfill()
})
},
}
)

const componentFile = 'app/editor-links/component.js'
const fileContent = await next.readFile(componentFile)

await session.patch(
componentFile,
fileContent.replace(
"// import { useState } from 'react'",
"import { useState } from 'react'"
)
)

expect(await session.hasRedbox(true)).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./app/editor-links/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:1]
1 | import { useState } from 'react'
: ^^^^^^^^
2 | export default function Component() {
3 | return <div>Component</div>
4 | }
\`----
Maybe one of these should be marked as a client entry with \\"use client\\":
app/editor-links/component.js
app/editor-links/page.js"
`)

await browser.waitForElementByCss('[data-with-open-in-editor-link]')
const collapsedFrameworkGroups = await browser.elementsByCss(
'[data-with-open-in-editor-link]'
)
for (const collapsedFrameworkButton of collapsedFrameworkGroups) {
await collapsedFrameworkButton.click()
}

await check(() => editorRequestsCount, /2/)

await cleanup()
})
}
)

0 comments on commit 39e1670

Please sign in to comment.