Skip to content

Commit

Permalink
Revert "Update split chunk handling for edge/node" (vercel#62313)
Browse files Browse the repository at this point in the history
We have a reproduction of OOMs still occurring with this chunking so
going to revert while we investigate further

x-ref:
vercel#51298 (comment)

Reverts vercel#62205

Closes NEXT-2548
  • Loading branch information
ijjk authored Feb 21, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 0b9d635 commit fc0f94f
Showing 5 changed files with 46 additions and 52 deletions.
63 changes: 21 additions & 42 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
@@ -715,15 +715,11 @@ export default async function getBaseWebpackConfig(
// Packages which will be split into the 'framework' chunk.
// Only top-level packages are included, e.g. nested copies like
// 'node_modules/meow/node_modules/object-assign' are not included.
const nextFrameworkPaths: string[] = []
const topLevelFrameworkPaths: string[] = []
const visitedFrameworkPackages = new Set<string>()

// Adds package-paths of dependencies recursively
const addPackagePath = (
packageName: string,
relativeToPath: string,
paths: string[]
) => {
const addPackagePath = (packageName: string, relativeToPath: string) => {
try {
if (visitedFrameworkPackages.has(packageName)) {
return
@@ -742,11 +738,11 @@ export default async function getBaseWebpackConfig(
const directory = path.join(packageJsonPath, '../')

// Returning from the function in case the directory has already been added and traversed
if (paths.includes(directory)) return
paths.push(directory)
if (topLevelFrameworkPaths.includes(directory)) return
topLevelFrameworkPaths.push(directory)
const dependencies = require(packageJsonPath).dependencies || {}
for (const name of Object.keys(dependencies)) {
addPackagePath(name, directory, paths)
addPackagePath(name, directory)
}
} catch (_) {
// don't error on failing to resolve framework packages
@@ -763,9 +759,8 @@ export default async function getBaseWebpackConfig(
]
: []),
]) {
addPackagePath(packageName, dir, topLevelFrameworkPaths)
addPackagePath(packageName, dir)
}
addPackagePath('next', dir, nextFrameworkPaths)

const crossOrigin = config.crossOrigin

@@ -918,7 +913,6 @@ export default async function getBaseWebpackConfig(
splitChunks: (():
| Required<webpack.Configuration>['optimization']['splitChunks']
| false => {
// server chunking
if (dev) {
if (isNodeServer) {
/*
@@ -969,6 +963,21 @@ export default async function getBaseWebpackConfig(
return false
}

if (isNodeServer) {
return {
filename: '[name].js',
chunks: 'all',
minChunks: 2,
}
}

if (isEdgeServer) {
return {
filename: 'edge-chunks/[name].js',
minChunks: 2,
}
}

const frameworkCacheGroup = {
chunks: 'all' as const,
name: 'framework',
@@ -987,22 +996,6 @@ export default async function getBaseWebpackConfig(
// becoming a part of the commons chunk)
enforce: true,
}

const nextRuntimeCacheGroup = {
chunks: 'all' as const,
name: 'next-runtime',
test(module: any) {
const resource = module.nameForCondition?.()
return resource
? nextFrameworkPaths.some((pkgPath) =>
resource.startsWith(pkgPath)
)
: false
},
priority: 30,
enforce: true,
}

const libCacheGroup = {
test(module: {
size: Function
@@ -1044,20 +1037,6 @@ export default async function getBaseWebpackConfig(
minChunks: 1,
reuseExistingChunk: true,
}

if (isNodeServer || isEdgeServer) {
return {
filename: `${isEdgeServer ? 'edge-chunks/' : ''}[name].js`,
cacheGroups: {
nextRuntime: nextRuntimeCacheGroup,
framework: frameworkCacheGroup,
lib: libCacheGroup,
},
minChunks: 2,
}
}

// client chunking
const cssCacheGroup = {
test: /\.(css|sass|scss)$/i,
chunks: 'all' as const,
Original file line number Diff line number Diff line change
@@ -98,6 +98,16 @@ export class TerserPlugin {
return false
}

// don't minify _middleware as it can break in some cases
// and doesn't provide too much of a benefit as it's server-side
if (
name.match(
/(edge-runtime-webpack\.js|edge-chunks|middleware\.js$)/
)
) {
return false
}

const { info } = res

// Skip double minimize assets from child compilation
4 changes: 2 additions & 2 deletions test/e2e/middleware-trailing-slash/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -111,11 +111,11 @@ describe('Middleware Runtime trailing slash', () => {
)
expect(manifest.middleware).toEqual({
'/': {
files: expect.arrayContaining([
files: [
'prerender-manifest.js',
'server/edge-runtime-webpack.js',
'server/middleware.js',
]),
],
name: 'middleware',
page: '/',
matchers: [{ regexp: '^/.*$', originalSource: '/:path*' }],
16 changes: 8 additions & 8 deletions test/e2e/switchable-runtime/index.test.ts
Original file line number Diff line number Diff line change
@@ -187,10 +187,10 @@ describe('Switchable runtime', () => {
expect(manifest).toMatchObject({
functions: {
'/api/hello': {
files: expect.arrayContaining([
files: [
'server/edge-runtime-webpack.js',
'server/pages/api/hello.js',
]),
],
name: 'pages/api/hello',
page: '/api/hello',
matchers: [
@@ -199,10 +199,10 @@ describe('Switchable runtime', () => {
wasm: [],
},
'/api/edge': {
files: expect.arrayContaining([
files: [
'server/edge-runtime-webpack.js',
'server/pages/api/edge.js',
]),
],
name: 'pages/api/edge',
page: '/api/edge',
matchers: [
@@ -621,11 +621,11 @@ describe('Switchable runtime', () => {
expect(manifest).toMatchObject({
functions: {
'/api/hello': {
files: expect.arrayContaining([
files: [
'prerender-manifest.js',
'server/edge-runtime-webpack.js',
'server/pages/api/hello.js',
]),
],
name: 'pages/api/hello',
page: '/api/hello',
matchers: [
@@ -634,11 +634,11 @@ describe('Switchable runtime', () => {
wasm: [],
},
'/api/edge': {
files: expect.arrayContaining([
files: [
'prerender-manifest.js',
'server/edge-runtime-webpack.js',
'server/pages/api/edge.js',
]),
],
name: 'pages/api/edge',
page: '/api/edge',
matchers: [
5 changes: 5 additions & 0 deletions test/integration/build-trace-extra-entries/test/index.test.js
Original file line number Diff line number Diff line change
@@ -50,6 +50,11 @@ describe('build trace with extra entries', () => {
expect(appTrace.files.some((file) => file.endsWith('hello.json'))).toBe(
true
)
expect(
appTrace.files.filter(
(file) => file.includes('chunks') && file.endsWith('.js')
).length
).toBe(0)

expect(
indexTrace.files.filter(

0 comments on commit fc0f94f

Please sign in to comment.