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.

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

Externalize next-server from Server Builds #11819

Merged
merged 5 commits into from
Apr 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 62 additions & 22 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,24 +497,32 @@ export default async function getBaseWebpackConfig(
return callback()
}

// make sure we don't externalize anything that is
// supposed to be transpiled
if (babelIncludeRegexes.some(r => r.test(request))) {
return callback()
}
// We need to externalize internal requests for files intended to
// not be bundled.

const isLocal: boolean =
request.startsWith('.') ||
// Always check for unix-style path, as webpack sometimes
// normalizes as posix.
path.posix.isAbsolute(request) ||
// When on Windows, we also want to check for Windows-specific
// absolute paths.
(process.platform === 'win32' && path.win32.isAbsolute(request))
const isLikelyNextExternal =
isLocal && /[/\\]next-server[/\\]/.test(request)

// Relative requires don't need custom resolution, because they
// are relative to requests we've already resolved here.
// Absolute requires (require('/foo')) are extremely uncommon, but
// also have no need for customization as they're already resolved.
if (request.startsWith('.') || request.startsWith('/')) {
if (isLocal && !isLikelyNextExternal) {
return callback()
}

// Resolve the import with the webpack provided context, this
// ensures we're resolving the correct version when multiple
// exist.
let res
let res: string
try {
res = resolveRequest(request, `${context}/`)
} catch (err) {
Expand All @@ -530,20 +538,36 @@ export default async function getBaseWebpackConfig(
return callback()
}

// Bundled Node.js code is relocated without its node_modules tree.
// This means we need to make sure its request resolves to the same
// package that'll be available at runtime. If it's not identical,
// we need to bundle the code (even if it _should_ be external).
let baseRes
try {
baseRes = resolveRequest(request, `${dir}/`)
} catch (err) {}
let isNextExternal: boolean = false
if (isLocal) {
isNextExternal = /next[/\\]dist[/\\]next-server[/\\]/.test(res)
if (!isNextExternal) {
return callback()
}
}

// Same as above: if the package, when required from the root,
// would be different from what the real resolution would use, we
// cannot externalize it.
if (baseRes !== res) {
return callback()
// `isNextExternal` special cases Next.js' internal requires that
// should not be bundled. We need to skip the base resolve routine
// to prevent it from being bundled (assumes Next.js version cannot
// mismatch).
if (!isNextExternal) {
// Bundled Node.js code is relocated without its node_modules tree.
// This means we need to make sure its request resolves to the same
// package that'll be available at runtime. If it's not identical,
// we need to bundle the code (even if it _should_ be external).
let baseRes: string | null
try {
baseRes = resolveRequest(request, `${dir}/`)
} catch (err) {
baseRes = null
}

// Same as above: if the package, when required from the root,
// would be different from what the real resolution would use, we
// cannot externalize it.
if (baseRes !== res) {
return callback()
}
}

// Default pages have to be transpiled
Expand All @@ -566,8 +590,24 @@ export default async function getBaseWebpackConfig(

// Anything else that is standard JavaScript within `node_modules`
// can be externalized.
if (res.match(/node_modules[/\\].*\.js$/)) {
return callback(undefined, `commonjs ${request}`)
if (isNextExternal || res.match(/node_modules[/\\].*\.js$/)) {
const externalRequest = isNextExternal
? // Generate Next.js external import
path.posix.join(
'next',
'dist',
path
.relative(
// Root of Next.js package:
path.join(__dirname, '..'),
res
)
// Windows path normalization
.replace(/\\/g, '/')
)
: request

return callback(undefined, `commonjs ${externalRequest}`)
}

// Default behavior: bundle the code!
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions test/integration/externalize-next-server/app/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "externalize-next-server-app",
"version": "1.0.0",
"main": "index.js",
"license": "MIT"
}
12 changes: 12 additions & 0 deletions test/integration/externalize-next-server/app/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import MyComp from 'comps'

const Page = () => (
<>
<p>Hello {typeof window}</p>
<MyComp />
</>
)

Page.getInitialProps = () => ({})

export default Page
35 changes: 35 additions & 0 deletions test/integration/externalize-next-server/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* eslint-env jest */
/* global jasmine */
import fs from 'fs-extra'
import path from 'path'
import { nextBuild } from 'next-test-utils'
import escapeStringRegexp from 'escape-string-regexp'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1

const appDir = path.join(__dirname, '../app')
let buildId

describe('externalize next/dist/next-server', () => {
beforeAll(async () => {
await nextBuild(appDir)
buildId = await fs.readFile(path.join(appDir, '.next/BUILD_ID'), 'utf8')
})

it('Does not bundle next/dist/next-server/lib/head.js in _error', async () => {
const content = await fs.readFile(
path.join(appDir, '.next/server/static', buildId, 'pages/_error.js'),
'utf8'
)
expect(content).toMatch(
new RegExp(
'^' +
escapeStringRegexp(
`module.exports = require("next/dist/next-server/lib/head.js");`
) +
'$',
'm'
)
)
})
})