Skip to content

Commit

Permalink
use a separate webpack runtime for middleware (#33134)
Browse files Browse the repository at this point in the history
it should not leak into the client runtime

cc @javivelasco 



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
  • Loading branch information
sokra authored Jan 10, 2022
1 parent 8ae08b9 commit f0ad19a
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 41 deletions.
7 changes: 6 additions & 1 deletion packages/next/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import { NextConfigComplete } from '../server/config-shared'
import { isCustomErrorPage, isFlightPage, isReservedPage } from './utils'
import { ssrEntries } from './webpack/plugins/middleware-plugin'
import type { webpack5 } from 'next/dist/compiled/webpack/webpack'
import { MIDDLEWARE_SSR_RUNTIME_WEBPACK } from '../shared/lib/constants'
import {
MIDDLEWARE_RUNTIME_WEBPACK,
MIDDLEWARE_SSR_RUNTIME_WEBPACK,
} from '../shared/lib/constants'

type ObjectValue<T> = T extends { [key: string]: infer V } ? V : never
export type PagesMapping = {
Expand Down Expand Up @@ -291,6 +294,8 @@ export function finalizeEntrypoint({
name: ['_ENTRIES', `middleware_[name]`],
type: 'assign',
},
runtime: MIDDLEWARE_RUNTIME_WEBPACK,
asyncChunks: false,
...entry,
}
return middlewareEntry
Expand Down
18 changes: 11 additions & 7 deletions packages/next/build/webpack/plugins/middleware-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
MIDDLEWARE_FLIGHT_MANIFEST,
MIDDLEWARE_BUILD_MANIFEST,
MIDDLEWARE_REACT_LOADABLE_MANIFEST,
MIDDLEWARE_RUNTIME_WEBPACK,
} from '../../../shared/lib/constants'
import { MIDDLEWARE_ROUTE } from '../../../lib/constants'
import { nonNullable } from '../../../lib/non-nullable'
Expand Down Expand Up @@ -89,13 +90,7 @@ export default class MiddlewarePlugin {
`server/${MIDDLEWARE_REACT_LOADABLE_MANIFEST}.js`,
...entryFiles.map((file) => 'server/' + file),
].filter(nonNullable)
: entryFiles.map((file: string) =>
// we need to use the unminified version of the webpack runtime,
// remove if we do start minifying middleware chunks
file.startsWith('static/chunks/webpack-')
? file.replace('webpack-', 'webpack-middleware-')
: file
)
: entryFiles.map((file: string) => file)

middlewareManifest.middleware[location] = {
env: envPerRoute.get(entrypoint.name) || [],
Expand Down Expand Up @@ -130,6 +125,15 @@ export default class MiddlewarePlugin {
compiler.hooks.compilation.tap(
PLUGIN_NAME,
(compilation, { normalModuleFactory }) => {
compilation.hooks.afterChunks.tap(PLUGIN_NAME, () => {
const middlewareRuntimeChunk = compilation.namedChunks.get(
MIDDLEWARE_RUNTIME_WEBPACK
)
if (middlewareRuntimeChunk) {
middlewareRuntimeChunk.filenameTemplate = 'server/[name].js'
}
})

const envPerRoute = new Map<string, string[]>()

compilation.hooks.afterOptimizeModules.tap(PLUGIN_NAME, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ export class TerserPlugin {
terserSpan.setAttribute('compilationName', compilation.name)

return terserSpan.traceAsyncFn(async () => {
let webpackAsset = ''
let hasMiddleware = false
let numberOfAssetsForMinify = 0
const assetsList = Object.keys(assets)

Expand All @@ -99,15 +97,14 @@ export class TerserPlugin {
return false
}

// remove below if we start minifying middleware chunks
if (name.startsWith('static/chunks/webpack-')) {
webpackAsset = name
}

// 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(/(middleware-chunks|_middleware\.js$)/)) {
hasMiddleware = true
if (
name.match(
/(middleware-runtime\.js|middleware-chunks|_middleware\.js$)/
)
) {
return false
}

const { info } = res
Expand Down Expand Up @@ -145,17 +142,6 @@ export class TerserPlugin {
})
)

if (hasMiddleware && webpackAsset) {
// emit a separate version of the webpack
// runtime for the middleware
const asset = compilation.getAsset(webpackAsset)
compilation.emitAsset(
webpackAsset.replace('webpack-', 'webpack-middleware-'),
asset.source,
{}
)
}

const numberOfWorkers = Math.min(
numberOfAssetsForMinify,
optimizeOptions.availableNumberOfCores
Expand Down
1 change: 1 addition & 0 deletions packages/next/shared/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const CLIENT_STATIC_FILES_RUNTIME_WEBPACK = `webpack`
export const CLIENT_STATIC_FILES_RUNTIME_POLYFILLS_SYMBOL = Symbol(`polyfills`)
// server/middleware-flight-runtime.js
export const MIDDLEWARE_SSR_RUNTIME_WEBPACK = 'middleware-ssr-runtime'
export const MIDDLEWARE_RUNTIME_WEBPACK = 'middleware-runtime'
export const TEMPORARY_REDIRECT_STATUS = 307
export const PERMANENT_REDIRECT_STATUS = 308
export const STATIC_PROPS_ID = '__N_SSG'
Expand Down
16 changes: 6 additions & 10 deletions test/integration/middleware/core/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,12 @@ describe('Middleware base tests', () => {
)
for (const key of Object.keys(manifest.middleware)) {
const middleware = manifest.middleware[key]
expect(
middleware.files.some((file) => file.includes('webpack-middleware'))
).toBe(true)
expect(
middleware.files.filter(
(file) =>
file.startsWith('static/chunks/') &&
!file.startsWith('static/chunks/webpack-middleware')
).length
).toBe(0)
expect(middleware.files).toContainEqual(
expect.stringContaining('middleware-runtime')
)
expect(middleware.files).not.toContainEqual(
expect.stringContaining('static/chunks/')
)
}
})
})
Expand Down
6 changes: 3 additions & 3 deletions test/production/required-server-files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ describe('should set-up next', () => {
})

it('should output middleware correctly', async () => {
// the middleware-runtime is located in .next/static/chunks so ensure
// the folder is present
expect(
await fs.pathExists(join(next.testDir, 'standalone/.next/static/chunks'))
await fs.pathExists(
join(next.testDir, 'standalone/.next/server/middleware-runtime.js')
)
).toBe(true)
expect(
await fs.pathExists(
Expand Down

0 comments on commit f0ad19a

Please sign in to comment.