Skip to content

Commit

Permalink
Warn/revert custom devtool in development mode (#14285)
Browse files Browse the repository at this point in the history
Warn users and revert their `devtool` when they manually change the `devtool` in development mode. For this addition, I check to ensure the `devtool` is custom (i.e. different than what is set by Next) and has a value (`false` is fine as a custom `devtool`!).

As described in [this issue (13963)](#13963), changing the `devtool` in development mode can cause issues with performance.

Fixes #13963
  • Loading branch information
jamesmosier authored Jun 24, 2020
1 parent afa9bab commit 435bf65
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 2 deletions.
20 changes: 20 additions & 0 deletions errors/improper-devtool.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Improper webpack `devtool` used in development mode

#### Why This Error Occurred

Next.js chooses the most optimal `devtool` for use with webpack. Changing the `devtool` in development mode will cause severe performance regressions with your application.

#### Possible Ways to Fix It

Please remove the custom `devtool` override or only apply it to production builds in your `next.config.js`.

```js
module.exports = {
webpack: (config, options) => {
if (!options.dev) {
config.devtool = options.isServer ? false : 'your-custom-devtool'
}
return config
},
}
```
21 changes: 19 additions & 2 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { codeFrameColumns } from '@babel/code-frame'
import ReactRefreshWebpackPlugin from '@next/react-refresh-utils/ReactRefreshWebpackPlugin'
import crypto from 'crypto'
import { readFileSync } from 'fs'
import chalk from 'next/dist/compiled/chalk'
import TerserPlugin from 'next/dist/compiled/terser-webpack-plugin'
import path from 'path'
import webpack from 'webpack'
import type { Configuration } from 'webpack'
import {
DOT_NEXT_ALIAS,
NEXT_PROJECT_ROOT,
Expand All @@ -21,6 +23,7 @@ import {
SERVERLESS_DIRECTORY,
SERVER_DIRECTORY,
} from '../next-server/lib/constants'
import { execOnce } from '../next-server/lib/utils'
import { findPageFile } from '../server/lib/find-page-file'
import { WebpackEntrypoints } from './entries'
import {
Expand All @@ -44,12 +47,11 @@ import { ReactLoadablePlugin } from './webpack/plugins/react-loadable-plugin'
import { ServerlessPlugin } from './webpack/plugins/serverless-plugin'
import WebpackConformancePlugin, {
DuplicatePolyfillsConformanceCheck,
GranularChunksConformanceCheck,
MinificationConformanceCheck,
ReactSyncScriptsConformanceCheck,
GranularChunksConformanceCheck,
} from './webpack/plugins/webpack-conformance-plugin'
import { WellKnownErrorsPlugin } from './webpack/plugins/wellknown-errors-plugin'
import { codeFrameColumns } from '@babel/code-frame'

type ExcludesFalse = <T>(x: T | false) => x is T

Expand All @@ -61,6 +63,15 @@ const escapePathVariables = (value: any) => {
: value
}

const devtoolRevertWarning = execOnce((devtool: Configuration['devtool']) => {
console.warn(
chalk.yellow.bold('Warning: ') +
chalk.bold(`Reverting webpack devtool to '${devtool}'.\n`) +
'Changing the webpack devtool in development mode will cause severe performance regressions.\n' +
'Read more: https://err.sh/next.js/improper-devtool'
)
})

function parseJsonFile(filePath: string) {
const JSON5 = require('next/dist/compiled/json5')
const contents = readFileSync(filePath, 'utf8')
Expand Down Expand Up @@ -1002,6 +1013,7 @@ export default async function getBaseWebpackConfig(
productionBrowserSourceMaps,
})

let originalDevtool = webpackConfig.devtool
if (typeof config.webpack === 'function') {
webpackConfig = config.webpack(webpackConfig, {
dir,
Expand All @@ -1014,6 +1026,11 @@ export default async function getBaseWebpackConfig(
webpack,
})

if (dev && originalDevtool !== webpackConfig.devtool) {
webpackConfig.devtool = originalDevtool
devtoolRevertWarning(originalDevtool)
}

if (typeof (webpackConfig as any).then === 'function') {
console.warn(
'> Promise returned in next config. https://err.sh/vercel/next.js/promise-in-next-config'
Expand Down
6 changes: 6 additions & 0 deletions test/integration/config-devtool-dev/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
webpack: (config) => {
config.devtool = false
return config
},
}
8 changes: 8 additions & 0 deletions test/integration/config-devtool-dev/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { useEffect } from 'react'

export default function Index(props) {
useEffect(() => {
throw new Error('this should render')
}, [])
return <div>Index Page</div>
}
58 changes: 58 additions & 0 deletions test/integration/config-devtool-dev/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/* eslint-env jest */

import {
check,
findPort,
getRedboxSource,
hasRedbox,
killApp,
launchApp,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import { join } from 'path'

jest.setTimeout(1000 * 30)

const appDir = join(__dirname, '../')

describe('devtool set in developmemt mode in next config', () => {
it('should warn and revert when a devtool is set in development mode', async () => {
let stderr = ''

const appPort = await findPort()
const app = await launchApp(appDir, appPort, {
env: { __NEXT_TEST_WITH_DEVTOOL: true },
onStderr(msg) {
stderr += msg || ''
},
})

const found = await check(
() => stderr,
/Reverting webpack devtool to /,
false
)

const browser = await webdriver(appPort, '/')
expect(await hasRedbox(browser)).toBe(true)
if (process.platform === 'win32') {
// TODO: add win32 snapshot
} else {
expect(await getRedboxSource(browser)).toMatchInlineSnapshot(`
"pages/index.js (5:10) @ eval
3 | export default function Index(props) {
4 | useEffect(() => {
> 5 | throw new Error('this should render')
| ^
6 | }, [])
7 | return <div>Index Page</div>
8 | }"
`)
}
await browser.close()

await killApp(app)
expect(found).toBeTruthy()
})
})

0 comments on commit 435bf65

Please sign in to comment.