From 9b09b92a1443c34a33c85c452e93dc93f3d966cf Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 19 Aug 2021 03:12:12 -0500 Subject: [PATCH] Ensure adding _app/_document HMRs correctly (#28279) This is a follow-up to https://github.com/vercel/next.js/pull/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: https://github.com/vercel/next.js/issues/27888 --- packages/next/build/entries.ts | 20 +++- packages/next/build/index.ts | 4 +- packages/next/build/webpack-config.ts | 78 ++++++------- .../build/webpack/config/blocks/css/index.ts | 6 +- .../webpack/config/blocks/css/messages.ts | 4 +- packages/next/build/webpack/config/index.ts | 2 +- packages/next/build/webpack/config/utils.ts | 2 +- packages/next/server/dev/hot-reloader.ts | 4 +- .../app-document-add-hmr/pages/index.js | 3 + .../app-document-add-hmr/test/index.test.js | 110 ++++++++++++++++++ 10 files changed, 174 insertions(+), 59 deletions(-) create mode 100644 test/integration/app-document-add-hmr/pages/index.js create mode 100644 test/integration/app-document-add-hmr/test/index.test.js diff --git a/packages/next/build/entries.ts b/packages/next/build/entries.ts index b9705c2d0a964a8..7ab24a2f4135f53 100644 --- a/packages/next/build/entries.ts +++ b/packages/next/build/entries.ts @@ -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( @@ -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 } diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 294bdcf1e621e65..bc8657328fa3150 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -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(() => diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 881b5169ae8f8fd..12106b2e459581e 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -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' @@ -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 { @@ -444,6 +421,9 @@ 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' ) @@ -451,6 +431,34 @@ export default async function getBaseWebpackConfig( '../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 @@ -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, @@ -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 || '', diff --git a/packages/next/build/webpack/config/blocks/css/index.ts b/packages/next/build/webpack/config/blocks/css/index.ts index 02f44cf5c70a58e..56dae4c11841d3e 100644 --- a/packages/next/build/webpack/config/blocks/css/index.ts +++ b/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' @@ -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(), }, }, }, diff --git a/packages/next/build/webpack/config/blocks/css/messages.ts b/packages/next/build/webpack/config/blocks/css/messages.ts index 787d46750e5ec32..c48912399ed3fbf 100644 --- a/packages/next/build/webpack/config/blocks/css/messages.ts +++ b/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 ' )}. 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` } diff --git a/packages/next/build/webpack/config/index.ts b/packages/next/build/webpack/config/index.ts index 033b4c93e9b9ca9..82186ac809de212 100644 --- a/packages/next/build/webpack/config/index.ts +++ b/packages/next/build/webpack/config/index.ts @@ -18,7 +18,7 @@ export async function build( isCraCompat, }: { rootDirectory: string - customAppFile: string | null + customAppFile: RegExp isDevelopment: boolean isServer: boolean assetPrefix: string diff --git a/packages/next/build/webpack/config/utils.ts b/packages/next/build/webpack/config/utils.ts index a7f856088bc4265..c596727401b2bc0 100644 --- a/packages/next/build/webpack/config/utils.ts +++ b/packages/next/build/webpack/config/utils.ts @@ -3,7 +3,7 @@ import { NextConfigComplete } from '../../../server/config-shared' export type ConfigurationContext = { rootDirectory: string - customAppFile: string | null + customAppFile: RegExp isDevelopment: boolean isProduction: boolean diff --git a/packages/next/server/dev/hot-reloader.ts b/packages/next/server/dev/hot-reloader.ts index ff29138ce959702..81fcdefd70eec38 100644 --- a/packages/next/server/dev/hot-reloader.ts +++ b/packages/next/server/dev/hot-reloader.ts @@ -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, diff --git a/test/integration/app-document-add-hmr/pages/index.js b/test/integration/app-document-add-hmr/pages/index.js new file mode 100644 index 000000000000000..08263e34c35fd22 --- /dev/null +++ b/test/integration/app-document-add-hmr/pages/index.js @@ -0,0 +1,3 @@ +export default function Page() { + return

index page

+} diff --git a/test/integration/app-document-add-hmr/test/index.test.js b/test/integration/app-document-add-hmr/test/index.test.js new file mode 100644 index 000000000000000..bf7b42adbe951d8 --- /dev/null +++ b/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 ( + <> +

custom _app

+ + + ) + } + ` + ) + + 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 ( + + + +

custom _document

+
+ + + + ) + } + } + + 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) + } + }) +})