Skip to content

Commit

Permalink
fix: exclude _app from _document bundle for pages
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi committed Aug 28, 2023
1 parent 8ed492f commit 5a81930
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 29 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ export function createPagesMapping({
if (isDev) {
delete pages['/_app']
delete pages['/_error']
delete pages['/_document']
// delete pages['/_document']
}

// In development we always alias these to allow Webpack to fallback to
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ export default async function getBaseWebpackConfig(
alias: {
// Alias next/head component to noop for RSC
[require.resolve('next/head')]: require.resolve(
'next/dist/client/components/noop-head'
'next/dist/client/components/noop'
),
// Alias next/dynamic
[require.resolve('next/dynamic')]: require.resolve(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { warnOnce } from '../../../../shared/lib/utils/warn-once'
import { getRSCModuleInformation } from '../../../analysis/get-page-static-info'
import { getModuleBuildInfo } from '../get-module-build-info'

const noopHeadPath = require.resolve('next/dist/client/components/noop-head')
const noopComponentPath = require.resolve('next/dist/client/components/noop')
// For edge runtime it will be aliased to esm version by webpack
const MODULE_PROXY_PATH =
'next/dist/build/webpack/loaders/next-flight-loader/module-proxy'
Expand Down Expand Up @@ -91,7 +91,7 @@ export { e${cnt++} as ${ref} };`
}

if (buildInfo.rsc?.type !== RSC_MODULE_TYPES.client) {
if (noopHeadPath === this.resourcePath) {
if (noopComponentPath === this.resourcePath) {
warnOnce(
`Warning: You're using \`next/head\` inside the \`app\` directory, please migrate to the Metadata API. See https://nextjs.org/docs/app/building-your-application/upgrading/app-router-migration#step-3-migrating-nexthead for more details.`
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { normalizePagePath } from '../../../../shared/lib/page-path/normalize-pa
import { decodeFromBase64, encodeToBase64 } from '../utils'
import { isInstrumentationHookFile } from '../../../worker'
import { loadEntrypoint } from '../../../load-entrypoint'
import { isInternalComponent } from '../../../../lib/is-internal-component'

type RouteLoaderOptionsPagesAPIInput = {
kind: RouteKind.PAGES_API
Expand Down Expand Up @@ -162,7 +163,10 @@ const loadPages = async (
let file = await loadEntrypoint('pages', {
VAR_USERLAND: absolutePagePath,
VAR_MODULE_DOCUMENT: absoluteDocumentPath,
VAR_MODULE_APP: absoluteAppPath,
VAR_MODULE_APP:
page === '/_document'
? 'next/dist/client/components/noop'
: absoluteAppPath,
VAR_DEFINITION_PAGE: normalizePagePath(page),
VAR_DEFINITION_PATHNAME: page,
})
Expand Down
3 changes: 0 additions & 3 deletions packages/next/src/client/components/noop-head.tsx

This file was deleted.

3 changes: 3 additions & 0 deletions packages/next/src/client/components/noop.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function NoopComponent() {
return null
}
22 changes: 1 addition & 21 deletions packages/next/src/server/dev/hot-reloader-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1222,31 +1222,11 @@ export default class HotReloader implements NextJsHotReloaderInterface {
return
}

// If _document.js didn't change we don't trigger a reload
// If _document.js didn't change we don't trigger a reload.
if (documentChunk.hash === this.serverPrevDocumentHash) {
return
}

// As document chunk will change if new app pages are joined,
// since react bundle is different it will effect the chunk hash.
// So we diff the chunk changes, if there's only new app page chunk joins,
// then we don't trigger a reload by checking pages/_document chunk change.
if (this.appDir) {
const chunkNames = new Set(compilation.namedChunks.keys())
const diffChunkNames = difference<string>(
this.serverChunkNames || new Set(),
chunkNames
)

if (
diffChunkNames.length === 0 ||
diffChunkNames.every((chunkName) => chunkName.startsWith('app/'))
) {
return
}
this.serverChunkNames = chunkNames
}

// Notify reload to reload the page, as _document.js was changed (different hash)
this.send('reloadPage')
this.serverPrevDocumentHash = documentChunk.hash || null
Expand Down
46 changes: 46 additions & 0 deletions test/development/pages-dir/custom-app-hmr/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'

createNextDescribe(
'custom-app-hmr',
{
files: __dirname,
},
({ next }) => {
it('should not do full reload when simply editing _app.js', async () => {
const customAppFilePath = 'pages/_app.js'
const browser = await next.browser('/')
await browser.eval('window.hmrConstantValue = "should-not-change"')

const customAppContent = await next.readFile(customAppFilePath)
const newCustomAppContent = customAppContent.replace(
'hmr text origin',
'hmr text changed'
)
await next.patchFile(customAppFilePath, newCustomAppContent)

await check(async () => {
const pText = await browser.elementByCss('h1').text()
expect(pText).toBe('hmr text changed')

// Should keep the value on window, which indicates there's no full reload
const hmrConstantValue = await browser.eval('window.hmrConstantValue')
expect(hmrConstantValue).toBe('should-not-change')

return 'success'
}, 'success')

await next.patchFile(customAppFilePath, customAppContent)
await check(async () => {
const pText = await browser.elementByCss('h1').text()
expect(pText).toBe('hmr text origin')

// Should keep the value on window, which indicates there's no full reload
const hmrConstantValue = await browser.eval('window.hmrConstantValue')
expect(hmrConstantValue).toBe('should-not-change')

return 'success'
}, 'success')
})
}
)
8 changes: 8 additions & 0 deletions test/development/pages-dir/custom-app-hmr/pages/_app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function App({ Component, pageProps }) {
return (
<>
<h1>hmr text origin</h1>
<Component {...pageProps} />
</>
)
}
3 changes: 3 additions & 0 deletions test/development/pages-dir/custom-app-hmr/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>index page</p>
}

0 comments on commit 5a81930

Please sign in to comment.