Skip to content

Commit

Permalink
Do not polyfill node built-in modules on edge functions (#36190)
Browse files Browse the repository at this point in the history
As the title. This is intended to be applied on both middleware and edge functions.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] 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`


Co-authored-by: Jiachi Liu <4800338+huozhi@users.noreply.github.com>
  • Loading branch information
nkzawa and huozhi authored Apr 25, 2022
1 parent e832007 commit a628256
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 35 deletions.
9 changes: 8 additions & 1 deletion packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ export default async function build(
if (result.errors.length > 5) {
result.errors.length = 5
}
const error = result.errors.join('\n\n')
let error = result.errors.join('\n\n')

console.error(chalk.red('Failed to compile.\n'))

Expand All @@ -739,6 +739,13 @@ export default async function build(
)
}

const breakingChangeIndex = error.indexOf(
'\n\nBREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.'
)
if (breakingChangeIndex >= 0) {
error = error.slice(0, breakingChangeIndex)
}

console.error(error)
console.error()

Expand Down
107 changes: 76 additions & 31 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -626,40 +626,10 @@ export default async function getBaseWebpackConfig(

setimmediate: 'next/dist/compiled/setimmediate',
},
...(targetWeb
...(isEdgeRuntime
? {
// Full list of old polyfills is accessible here:
// https://github.com/webpack/webpack/blob/2a0536cf510768111a3a6dceeb14cb79b9f59273/lib/ModuleNotFoundError.js#L13-L42
fallback: {
assert: require.resolve('next/dist/compiled/assert'),
buffer: require.resolve('next/dist/compiled/buffer/'),
constants: require.resolve(
'next/dist/compiled/constants-browserify'
),
crypto: require.resolve('next/dist/compiled/crypto-browserify'),
domain: require.resolve('next/dist/compiled/domain-browser'),
http: require.resolve('next/dist/compiled/stream-http'),
https: require.resolve('next/dist/compiled/https-browserify'),
os: require.resolve('next/dist/compiled/os-browserify'),
path: require.resolve('next/dist/compiled/path-browserify'),
punycode: require.resolve('next/dist/compiled/punycode'),
process: require.resolve('./polyfills/process'),
// Handled in separate alias
querystring: require.resolve('next/dist/compiled/querystring-es3'),
stream: require.resolve('next/dist/compiled/stream-browserify'),
string_decoder: require.resolve(
'next/dist/compiled/string_decoder'
),
sys: require.resolve('next/dist/compiled/util/'),
timers: require.resolve('next/dist/compiled/timers-browserify'),
tty: require.resolve('next/dist/compiled/tty-browserify'),
// Handled in separate alias
// url: require.resolve('url/'),
util: require.resolve('next/dist/compiled/util/'),
vm: require.resolve('next/dist/compiled/vm-browserify'),
zlib: require.resolve('next/dist/compiled/browserify-zlib'),
events: require.resolve('next/dist/compiled/events/'),
setImmediate: require.resolve('next/dist/compiled/setimmediate'),
},
}
: undefined),
Expand Down Expand Up @@ -1280,6 +1250,81 @@ export default async function getBaseWebpackConfig(
},
]
: []),
...(!isServer && !isEdgeRuntime
? [
{
oneOf: [
{
issuerLayer: 'middleware',
resolve: {
fallback: {
process: require.resolve('./polyfills/process'),
},
},
},
{
resolve: {
// Full list of old polyfills is accessible here:
// https://github.com/webpack/webpack/blob/2a0536cf510768111a3a6dceeb14cb79b9f59273/lib/ModuleNotFoundError.js#L13-L42
fallback: {
assert: require.resolve('next/dist/compiled/assert'),
buffer: require.resolve('next/dist/compiled/buffer/'),
constants: require.resolve(
'next/dist/compiled/constants-browserify'
),
crypto: require.resolve(
'next/dist/compiled/crypto-browserify'
),
domain: require.resolve(
'next/dist/compiled/domain-browser'
),
http: require.resolve('next/dist/compiled/stream-http'),
https: require.resolve(
'next/dist/compiled/https-browserify'
),
os: require.resolve('next/dist/compiled/os-browserify'),
path: require.resolve(
'next/dist/compiled/path-browserify'
),
punycode: require.resolve(
'next/dist/compiled/punycode'
),
process: require.resolve('./polyfills/process'),
// Handled in separate alias
querystring: require.resolve(
'next/dist/compiled/querystring-es3'
),
stream: require.resolve(
'next/dist/compiled/stream-browserify'
),
string_decoder: require.resolve(
'next/dist/compiled/string_decoder'
),
sys: require.resolve('next/dist/compiled/util/'),
timers: require.resolve(
'next/dist/compiled/timers-browserify'
),
tty: require.resolve(
'next/dist/compiled/tty-browserify'
),
// Handled in separate alias
// url: require.resolve('url/'),
util: require.resolve('next/dist/compiled/util/'),
vm: require.resolve('next/dist/compiled/vm-browserify'),
zlib: require.resolve(
'next/dist/compiled/browserify-zlib'
),
events: require.resolve('next/dist/compiled/events/'),
setImmediate: require.resolve(
'next/dist/compiled/setimmediate'
),
},
},
},
],
},
]
: []),
].filter(Boolean),
},
plugins: [
Expand Down
1 change: 0 additions & 1 deletion packages/next/server/web/sandbox/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ function createContext(options: {
File,
FormData,
process: {
...polyfills.process,
env: buildEnvironmentVariablesFrom(options.env),
},
ReadableStream,
Expand Down
3 changes: 1 addition & 2 deletions packages/next/server/web/sandbox/polyfills.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Crypto as WebCrypto } from 'next/dist/compiled/@peculiar/webcrypto'
import { CryptoKey } from 'next/dist/compiled/@peculiar/webcrypto'
import { v4 as uuid } from 'next/dist/compiled/uuid'
import processPolyfill from 'next/dist/compiled/process'

import crypto from 'crypto'

Expand All @@ -13,7 +12,7 @@ export function btoa(str: string) {
return Buffer.from(str, 'binary').toString('base64')
}

export { CryptoKey, processPolyfill as process }
export { CryptoKey }

export class Crypto extends WebCrypto {
// @ts-ignore Remove once types are updated and we deprecate node 12
Expand Down
2 changes: 2 additions & 0 deletions test/integration/middleware/core/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ describe('Middleware base tests', () => {
env: {
MIDDLEWARE_TEST: 'asdf',
},
// it's poflyfilled since there is the "process" module
// as a devDepencies of the next package
nextTick: 'function',
},
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { NextResponse } from 'next/server'
import { spawn } from 'child_process'

export async function middleware(request) {
console.log(spawn('ls', ['-lh', '/usr']))
return NextResponse.next()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { NextResponse } from 'next/server'
import { basename } from 'path'

export async function middleware(request) {
console.log(basename('/foo/bar/baz/asdf/quux.html'))
return NextResponse.next()
}
90 changes: 90 additions & 0 deletions test/integration/middleware/with-builtin-module/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/* eslint-env jest */

import stripAnsi from 'next/dist/compiled/strip-ansi'
import { join } from 'path'
import {
fetchViaHTTP,
findPort,
killApp,
launchApp,
nextBuild,
waitFor,
} from 'next-test-utils'

const context = {}
const WEBPACK_BREAKING_CHANGE = 'BREAKING CHANGE:'

jest.setTimeout(1000 * 60 * 2)
context.appDir = join(__dirname, '../')

describe('Middleware importing Node.js built-in module', () => {
function getModuleNotFound(name) {
return `Module not found: Can't resolve '${name}'`
}

function getBuiltinApisNotSupported(name) {
return `Native Node.js APIs are not supported in the Edge Runtime. Found \`${name}\` imported.\n`
}

describe('dev mode', () => {
let output = ''

beforeAll(async () => {
context.appPort = await findPort()
context.app = await launchApp(context.appDir, context.appPort, {
env: { __NEXT_TEST_WITH_DEVTOOL: 1 },
onStdout(msg) {
output += msg
},
onStderr(msg) {
output += msg
},
})
})

beforeEach(() => (output = ''))
afterAll(() => killApp(context.app))

it('shows error when importing path module', async () => {
const res = await fetchViaHTTP(context.appPort, '/using-path')
await waitFor(500)
expect(res.status).toBe(500)
expect(output).toContain(getModuleNotFound('path'))
expect(output).toContain(getBuiltinApisNotSupported('path'))
expect(stripAnsi(output)).toContain("import { basename } from 'path'")
expect(output).not.toContain(WEBPACK_BREAKING_CHANGE)
})

it('shows error when importing child_process module', async () => {
const res = await fetchViaHTTP(context.appPort, '/using-child-process')
await waitFor(500)
expect(res.status).toBe(500)
expect(output).toContain(getModuleNotFound('child_process'))
expect(output).toContain(getBuiltinApisNotSupported('child_process'))
expect(stripAnsi(output)).toContain(
"import { spawn } from 'child_process'"
)
expect(output).not.toContain(WEBPACK_BREAKING_CHANGE)
})
})

describe('production mode', () => {
let buildResult

beforeAll(async () => {
buildResult = await nextBuild(context.appDir, undefined, {
stderr: true,
stdout: true,
})
})

it('should have middleware error during build', () => {
expect(buildResult.stderr).toContain(getModuleNotFound('child_process'))
expect(buildResult.stderr).toContain(getModuleNotFound('path'))
expect(buildResult.stderr).toContain(
getBuiltinApisNotSupported('child_process')
)
expect(buildResult.stderr).not.toContain(WEBPACK_BREAKING_CHANGE)
})
})
})

0 comments on commit a628256

Please sign in to comment.