From ee50d4433a7e52a28fa32e6b5bef837291d597a3 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 18 Oct 2022 00:48:27 -0700 Subject: [PATCH] Fix app sc_client componet HMR server-side (#41510) This ensures we properly clear the `sc_client` component cache from `react-server-dom-webpack` in development so that HMR works properly after a refresh. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` Fixes: [slack thread](https://vercel.slack.com/archives/C043ANYDB24/p1666051202574509) --- .../nextjs-require-cache-hot-reloader.ts | 28 +++++++++--- packages/next/server/app-render.tsx | 4 +- test/e2e/app-dir/index.test.ts | 44 +++++++++++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts b/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts index bbea9145a5a6..a7321570c8db 100644 --- a/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts +++ b/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts @@ -68,15 +68,33 @@ export class NextJsRequireCacheHotReloader implements WebpackPluginInstance { ) deleteCache(runtimeChunkPath) }) + let hasAppPath = false // we need to make sure to clear all server entries from cache // since they can have a stale webpack-runtime cache // which needs to always be in-sync - const entries = [...compilation.entries.keys()].filter( - (entry) => - entry.toString().startsWith('pages/') || - entry.toString().startsWith('app/') - ) + const entries = [...compilation.entries.keys()].filter((entry) => { + const isAppPath = entry.toString().startsWith('app/') + hasAppPath = hasAppPath || isAppPath + return entry.toString().startsWith('pages/') || isAppPath + }) + + if (hasAppPath) { + // ensure we reset the cache for sc_server components + // loaded via react-server-dom-webpack + const reactWebpackModId = require.resolve( + 'next/dist/compiled/react-server-dom-webpack' + ) + const reactWebpackMod = require.cache[reactWebpackModId] + + if (reactWebpackMod) { + for (const child of reactWebpackMod.children) { + child.parent = null + delete require.cache[child.id] + } + } + delete require.cache[reactWebpackModId] + } entries.forEach((page) => { const outputPath = path.join( diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 90bc3845dea8..ea2667d848f4 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -8,7 +8,6 @@ import type { FontLoaderManifest } from '../build/webpack/plugins/font-loader-ma import React, { experimental_use as use } from 'react' import { ParsedUrlQuery } from 'querystring' -import { createFromReadableStream } from 'next/dist/compiled/react-server-dom-webpack' import { NextParsedUrlQuery } from './request-meta' import RenderResult from './render-result' import { @@ -271,6 +270,9 @@ function useFlightResponse( if (flightResponseRef.current !== null) { return flightResponseRef.current } + const { + createFromReadableStream, + } = require('next/dist/compiled/react-server-dom-webpack') const [renderStream, forwardStream] = readableStreamTee(req) const res = createFromReadableStream(renderStream, { diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 134dab910a10..534af0ebfe88 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1197,6 +1197,50 @@ describe('app dir', () => { }) if (isDev) { + it('should HMR correctly for client component', async () => { + const filePath = 'app/client-component-route/page.js' + const origContent = await next.readFile(filePath) + + try { + const browser = await webdriver(next.url, '/client-component-route') + + const ssrInitial = await renderViaHTTP( + next.url, + '/client-component-route' + ) + + expect(ssrInitial).toContain( + 'hello from app/client-component-route' + ) + + expect(await browser.elementByCss('p').text()).toContain( + 'hello from app/client-component-route' + ) + + await next.patchFile( + filePath, + origContent.replace('hello from', 'swapped from') + ) + + await check(() => browser.elementByCss('p').text(), /swapped from/) + + const ssrUpdated = await renderViaHTTP( + next.url, + '/client-component-route' + ) + expect(ssrUpdated).toContain('swapped from') + + await next.patchFile(filePath, origContent) + + await check(() => browser.elementByCss('p').text(), /hello from/) + expect( + await renderViaHTTP(next.url, '/client-component-route') + ).toContain('hello from') + } finally { + await next.patchFile(filePath, origContent) + } + }) + it('should throw an error when getServerSideProps is used', async () => { const pageFile = 'app/client-with-errors/get-server-side-props/page.js'