Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix reloading Client Component require.cache #38633

Merged
merged 3 commits into from Jul 14, 2022
Merged

Fix reloading Client Component require.cache #38633

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9f91a9
Fix reloading Client Component require.cache
timneutkens Jul 14, 2022
ed6c4fc
Remove unused import
timneutkens Jul 14, 2022
5d46ddf
Merge branch 'canary' into fix/client-component-ssr-entry-reload
timneutkens Jul 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 9 additions & 34 deletions packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts
View file
Open in desktop
Expand Up @@ -3,7 +3,6 @@ import { clearModuleContext } from '../../../server/web/sandbox'
import { realpathSync } from 'fs'
import path from 'path'
import isError from '../../../lib/is-error'
import { NEXT_CLIENT_SSR_ENTRY_SUFFIX } from '../../../shared/lib/constants'

type Compiler = webpack5.Compiler
type WebpackPluginInstance = webpack5.WebpackPluginInstance
Expand All @@ -21,18 +20,18 @@ function deleteCache(filePath: string) {
} catch (e) {
if (isError(e) && e.code !== 'ENOENT') throw e
}
const module = require.cache[filePath]
if (module) {
const mod = require.cache[filePath]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to ensure it does not conflict with Node.js module variable.

if (mod) {
// remove the child reference from the originModules
for (const originModule of originModules) {
const parent = require.cache[originModule]
if (parent) {
const idx = parent.children.indexOf(module)
const idx = parent.children.indexOf(mod)
if (idx >= 0) parent.children.splice(idx, 1)
}
}
// remove parent references from external modules
for (const child of module.children) {
for (const child of mod.children) {
child.parent = null
}
delete require.cache[filePath]
Expand All @@ -47,8 +46,6 @@ const PLUGIN_NAME = 'NextJsRequireCacheHotReloader'
export class NextJsRequireCacheHotReloader implements WebpackPluginInstance {
prevAssets: any = null
hasServerComponents: boolean
previousOutputPathsWebpack5: Set<string> = new Set()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a look at these and it turns out it only adds / removes and never reads the set so it was not in use. Same for the line below.

currentOutputPathsWebpack5: Set<string> = new Set()

constructor(opts: { hasServerComponents: boolean }) {
this.hasServerComponents = opts.hasServerComponents
Expand All @@ -57,30 +54,9 @@ export class NextJsRequireCacheHotReloader implements WebpackPluginInstance {
apply(compiler: Compiler) {
compiler.hooks.assetEmitted.tap(
PLUGIN_NAME,
(file, { targetPath, content }) => {
this.currentOutputPathsWebpack5.add(targetPath)
(_file, { targetPath, content }) => {
deleteCache(targetPath)
clearModuleContext(targetPath, content.toString('utf-8'))

if (
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checked this and it is calling assetEmitted when the file is written for .__sc_client__ so I'm not sure why it wouldn' have that before but this check is no longer needed, especially with the added compilation.entries() check: https://github.com/vercel/next.js/pull/38633/files#diff-d57600652ac21cc49a0603a849016288549cd5bc7386dca47eb5eed4b9579459R75

this.hasServerComponents &&
/^(app|pages)\//.test(file) &&
/\.js$/.test(targetPath)
) {
// Also clear the potential __sc_client__ cache.
// @TODO: Investigate why the client ssr bundle isn't emitted as an asset here.
const clientComponentsSSRTarget = targetPath.replace(
/\.js$/,
NEXT_CLIENT_SSR_ENTRY_SUFFIX + '.js'
)
if (deleteCache(clientComponentsSSRTarget)) {
this.currentOutputPathsWebpack5.add(clientComponentsSSRTarget)
clearModuleContext(
clientComponentsSSRTarget,
content.toString('utf-8')
)
}
}
}
)

Expand All @@ -96,8 +72,10 @@ export class NextJsRequireCacheHotReloader implements WebpackPluginInstance {
// 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/')
const entries = [...compilation.entries.keys()].filter(
(entry) =>
entry.toString().startsWith('pages/') ||
entry.toString().startsWith('app/')
)

entries.forEach((page) => {
Expand All @@ -108,8 +86,5 @@ export class NextJsRequireCacheHotReloader implements WebpackPluginInstance {
deleteCache(outputPath)
})
})

this.previousOutputPathsWebpack5 = new Set(this.currentOutputPathsWebpack5)
this.currentOutputPathsWebpack5.clear()
}
}