Skip to content

Commit

Permalink
Ensure adding _app/_document HMRs correctly (#28279)
Browse files Browse the repository at this point in the history
This is a follow-up to #28227 to ensure `_app` and `_document` HMR correctly when you start the dev server and then add `_app` and `_document`. 

## Bug

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

x-ref: #27888
  • Loading branch information
ijjk committed Aug 19, 2021
1 parent 53f0973 commit 9b09b92
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 59 deletions.
20 changes: 15 additions & 5 deletions packages/next/build/entries.ts
Expand Up @@ -17,7 +17,9 @@ type PagesMapping = {

export function createPagesMapping(
pagePaths: string[],
extensions: string[]
extensions: string[],
isWebpack5: boolean,
isDev: boolean
): PagesMapping {
const previousPages: PagesMapping = {}
const pages: PagesMapping = pagePaths.reduce(
Expand Down Expand Up @@ -45,10 +47,18 @@ export function createPagesMapping(
{}
)

pages['/_app'] = pages['/_app'] || 'next/dist/pages/_app'
pages['/_error'] = pages['/_error'] || 'next/dist/pages/_error'
pages['/_document'] = pages['/_document'] || 'next/dist/pages/_document'

// we alias these in development and allow webpack to
// allow falling back to the correct source file so
// that HMR can work properly when a file is added/removed
if (isWebpack5 && isDev) {
pages['/_app'] = `${PAGES_DIR_ALIAS}/_app`
pages['/_error'] = `${PAGES_DIR_ALIAS}/_error`
pages['/_document'] = `${PAGES_DIR_ALIAS}/_document`
} else {
pages['/_app'] = pages['/_app'] || 'next/dist/pages/_app'
pages['/_error'] = pages['/_error'] || 'next/dist/pages/_error'
pages['/_document'] = pages['/_document'] || 'next/dist/pages/_document'
}
return pages
}

Expand Down
4 changes: 3 additions & 1 deletion packages/next/build/index.ts
Expand Up @@ -255,7 +255,9 @@ export default async function build(

const mappedPages = nextBuildSpan
.traceChild('create-pages-mapping')
.traceFn(() => createPagesMapping(pagePaths, config.pageExtensions))
.traceFn(() =>
createPagesMapping(pagePaths, config.pageExtensions, isWebpack5, false)
)
const entrypoints = nextBuildSpan
.traceChild('create-entrypoints')
.traceFn(() =>
Expand Down
78 changes: 35 additions & 43 deletions packages/next/build/webpack-config.ts
Expand Up @@ -28,7 +28,6 @@ import {
} from '../shared/lib/constants'
import { execOnce } from '../shared/lib/utils'
import { NextConfigComplete } from '../server/config-shared'
import { findPageFile } from '../server/lib/find-page-file'
import { WebpackEntrypoints } from './entries'
import * as Log from './output/log'
import { build as buildConfiguration } from './webpack/config'
Expand Down Expand Up @@ -413,28 +412,6 @@ export default async function getBaseWebpackConfig(
resolvedBaseUrl = path.resolve(dir, jsConfig.compilerOptions.baseUrl)
}

let customAppFile: string | null = await findPageFile(
pagesDir,
'/_app',
config.pageExtensions
)
let customAppFileExt = customAppFile ? path.extname(customAppFile) : null
if (customAppFile) {
customAppFile = path.resolve(path.join(pagesDir, customAppFile))
}

let customDocumentFile: string | null = await findPageFile(
pagesDir,
'/_document',
config.pageExtensions
)
let customDocumentFileExt = customDocumentFile
? path.extname(customDocumentFile)
: null
if (customDocumentFile) {
customDocumentFile = path.resolve(path.join(pagesDir, customDocumentFile))
}

function getReactProfilingInProduction() {
if (reactProductionProfiling) {
return {
Expand All @@ -444,13 +421,44 @@ export default async function getBaseWebpackConfig(
}
}

// tell webpack where to look for _app and _document
// using aliases to allow falling back to the default
// version when removed or not present
const clientResolveRewrites = require.resolve(
'../shared/lib/router/utils/resolve-rewrites'
)
const clientResolveRewritesNoop = require.resolve(
'../shared/lib/router/utils/resolve-rewrites-noop'
)

const customAppAliases: { [key: string]: string[] } = {}
const customErrorAlias: { [key: string]: string[] } = {}
const customDocumentAliases: { [key: string]: string[] } = {}

if (dev && isWebpack5) {
customAppAliases[`${PAGES_DIR_ALIAS}/_app`] = [
...config.pageExtensions.reduce((prev, ext) => {
prev.push(path.join(pagesDir, `_app.${ext}`))
return prev
}, [] as string[]),
'next/dist/pages/_app.js',
]
customAppAliases[`${PAGES_DIR_ALIAS}/_error`] = [
...config.pageExtensions.reduce((prev, ext) => {
prev.push(path.join(pagesDir, `_error.${ext}`))
return prev
}, [] as string[]),
'next/dist/pages/_error.js',
]
customDocumentAliases[`${PAGES_DIR_ALIAS}/_document`] = [
...config.pageExtensions.reduce((prev, ext) => {
prev.push(path.join(pagesDir, `_document.${ext}`))
return prev
}, [] as string[]),
'next/dist/pages/_document.js',
]
}

const resolveConfig = {
// Disable .mjs for node_modules bundling
extensions: isServer
Expand All @@ -477,25 +485,9 @@ export default async function getBaseWebpackConfig(
alias: {
next: NEXT_PROJECT_ROOT,

// fallback to default _app when custom is removed
...(dev && customAppFileExt && isWebpack5
? {
[`${PAGES_DIR_ALIAS}/_app${customAppFileExt}`]: [
path.join(pagesDir, `_app${customAppFileExt}`),
'next/dist/pages/_app.js',
],
}
: {}),

// fallback to default _document when custom is removed
...(dev && customDocumentFileExt && isWebpack5
? {
[`${PAGES_DIR_ALIAS}/_document${customDocumentFileExt}`]: [
path.join(pagesDir, `_document${customDocumentFileExt}`),
'next/dist/pages/_document.js',
],
}
: {}),
...customAppAliases,
...customErrorAlias,
...customDocumentAliases,

[PAGES_DIR_ALIAS]: pagesDir,
[DOT_NEXT_ALIAS]: distDir,
Expand Down Expand Up @@ -1567,7 +1559,7 @@ export default async function getBaseWebpackConfig(

webpackConfig = await buildConfiguration(webpackConfig, {
rootDirectory: dir,
customAppFile,
customAppFile: new RegExp(path.join(pagesDir, `_app`)),
isDevelopment: dev,
isServer,
assetPrefix: config.assetPrefix || '',
Expand Down
6 changes: 1 addition & 5 deletions packages/next/build/webpack/config/blocks/css/index.ts
@@ -1,5 +1,4 @@
import curry from 'next/dist/compiled/lodash.curry'
import path from 'path'
import { webpack, isWebpack5 } from 'next/dist/compiled/webpack/webpack'
import MiniCssExtractPlugin from '../../../plugins/mini-css-extract-plugin'
import { loader, plugin } from '../../helpers'
Expand Down Expand Up @@ -275,10 +274,7 @@ export const css = curry(async function css(
use: {
loader: 'error-loader',
options: {
reason: getGlobalImportError(
ctx.customAppFile &&
path.relative(ctx.rootDirectory, ctx.customAppFile)
),
reason: getGlobalImportError(),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions packages/next/build/webpack/config/blocks/css/messages.ts
@@ -1,12 +1,12 @@
import chalk from 'chalk'

export function getGlobalImportError(file: string | null) {
export function getGlobalImportError() {
return `Global CSS ${chalk.bold(
'cannot'
)} be imported from files other than your ${chalk.bold(
'Custom <App>'
)}. Due to the Global nature of stylesheets, and to avoid conflicts, Please move all first-party global CSS imports to ${chalk.cyan(
file ? file : 'pages/_app.js'
'pages/_app.js'
)}. Or convert the import to Component-Level CSS (CSS Modules).\nRead more: https://nextjs.org/docs/messages/css-global`
}

Expand Down
2 changes: 1 addition & 1 deletion packages/next/build/webpack/config/index.ts
Expand Up @@ -18,7 +18,7 @@ export async function build(
isCraCompat,
}: {
rootDirectory: string
customAppFile: string | null
customAppFile: RegExp
isDevelopment: boolean
isServer: boolean
assetPrefix: string
Expand Down
2 changes: 1 addition & 1 deletion packages/next/build/webpack/config/utils.ts
Expand Up @@ -3,7 +3,7 @@ import { NextConfigComplete } from '../../../server/config-shared'

export type ConfigurationContext = {
rootDirectory: string
customAppFile: string | null
customAppFile: RegExp

isDevelopment: boolean
isProduction: boolean
Expand Down
4 changes: 3 additions & 1 deletion packages/next/server/dev/hot-reloader.ts
Expand Up @@ -261,7 +261,9 @@ export default class HotReloader {

const pages = createPagesMapping(
pagePaths.filter((i) => i !== null) as string[],
this.config.pageExtensions
this.config.pageExtensions,
this.isWebpack5,
true
)
const entrypoints = createEntrypoints(
pages,
Expand Down
3 changes: 3 additions & 0 deletions test/integration/app-document-add-hmr/pages/index.js
@@ -0,0 +1,3 @@
export default function Page() {
return <p>index page</p>
}
110 changes: 110 additions & 0 deletions test/integration/app-document-add-hmr/test/index.test.js
@@ -0,0 +1,110 @@
/* eslint-env jest */

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

jest.setTimeout(1000 * 60 * 2)

const appDir = join(__dirname, '../')
const appPage = join(appDir, 'pages/_app.js')
const indexPage = join(appDir, 'pages/index.js')
const documentPage = join(appDir, 'pages/_document.js')

let appPort
let app

describe('_app/_document add HMR', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))

it('should HMR when _app is added', async () => {
let indexContent = await fs.readFile(indexPage)
try {
const browser = await webdriver(appPort, '/')

const html = await browser.eval('document.documentElement.innerHTML')
expect(html).not.toContain('custom _app')
expect(html).toContain('index page')

await fs.writeFile(
appPage,
`
export default function MyApp({ Component, pageProps }) {
return (
<>
<p>custom _app</p>
<Component {...pageProps} />
</>
)
}
`
)

await check(async () => {
const html = await browser.eval('document.documentElement.innerHTML')
return html.includes('custom _app') && html.includes('index page')
? 'success'
: html
}, 'success')
} finally {
await fs.writeFile(indexPage, indexContent)
await fs.remove(appPage)
}
})

it('should HMR when _document is added', async () => {
let indexContent = await fs.readFile(indexPage)
try {
const browser = await webdriver(appPort, '/')

const html = await browser.eval('document.documentElement.innerHTML')
expect(html).not.toContain('custom _document')
expect(html).toContain('index page')

await fs.writeFile(
documentPage,
`
import Document, { Html, Head, Main, NextScript } from 'next/document'
class MyDocument extends Document {
static async getInitialProps(ctx) {
const initialProps = await Document.getInitialProps(ctx)
return { ...initialProps }
}
render() {
return (
<Html>
<Head />
<body>
<p>custom _document</p>
<Main />
<NextScript />
</body>
</Html>
)
}
}
export default MyDocument
`
)

await check(async () => {
const html = await browser.eval('document.documentElement.innerHTML')
return html.includes('custom _document') && html.includes('index page')
? 'success'
: html
}, 'success')
} finally {
await fs.writeFile(indexPage, indexContent)
await fs.remove(documentPage)
}
})
})

0 comments on commit 9b09b92

Please sign in to comment.