Do not polyfill node built-in modules on edge functions #36190

nkzawa · 2022-04-15T13:41:31Z

As the title. This is intended to be applied on both middleware and edge functions.

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

ijjk · 2022-04-15T14:04:06Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
buildDuration	21.9s	21.5s	-418ms
buildDurationCached	8.2s	8.4s	⚠️ +128ms
nodeModulesSize	481 MB	481 MB	⚠️ +1.95 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
/ failed reqs	0	0	✓
/ total time (seconds)	4.795	4.744	-0.05
/ avg req/sec	521.34	527	+5.66
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.197	2.168	-0.03
/error-in-render avg req/sec	1138.05	1152.92	+14.87

Client Bundles (main, webpack)

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
925.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	28.6 kB	28.6 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	72.3 kB	72.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
_app-HASH.js gzip	1.36 kB	1.36 kB	✓
_error-HASH.js gzip	192 B	192 B	✓
amp-HASH.js gzip	309 B	309 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	3.04 kB	3.04 kB	✓
head-HASH.js gzip	351 B	351 B	✓
hooks-HASH.js gzip	920 B	920 B	✓
image-HASH.js gzip	5.73 kB	5.73 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.36 kB	2.36 kB	✓
routerDirect..HASH.js gzip	320 B	320 B	✓
script-HASH.js gzip	392 B	392 B	✓
withRouter-HASH.js gzip	319 B	319 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16 kB	16 kB	✓

Client Build Manifests

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
_buildManifest.js gzip	461 B	461 B	✓
Overall change	461 B	461 B	✓

Rendered Page Sizes

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	525 B	525 B	✓
Overall change	1.6 kB	1.6 kB	✓

Default Build with SWC (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
buildDuration	24.4s	24.9s	⚠️ +464ms
buildDurationCached	8.3s	8.4s	⚠️ +54ms
nodeModulesSize	481 MB	481 MB	⚠️ +1.95 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
/ failed reqs	0	0	✓
/ total time (seconds)	4.823	4.801	-0.02
/ avg req/sec	518.33	520.69	+2.36
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.204	2.218	⚠️ +0.01
/error-in-render avg req/sec	1134.26	1127.38	⚠️ -6.88

Client Bundles (main, webpack)

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
925.HASH.js gzip	178 B	178 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	29.1 kB	29.1 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	72.9 kB	72.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	179 B	179 B	✓
amp-HASH.js gzip	313 B	313 B	✓
css-HASH.js gzip	325 B	325 B	✓
dynamic-HASH.js gzip	3.02 kB	3.02 kB	✓
head-HASH.js gzip	351 B	351 B	✓
hooks-HASH.js gzip	921 B	921 B	✓
image-HASH.js gzip	5.78 kB	5.78 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	2.44 kB	2.44 kB	✓
routerDirect..HASH.js gzip	322 B	322 B	✓
script-HASH.js gzip	393 B	393 B	✓
withRouter-HASH.js gzip	317 B	317 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16.1 kB	16.1 kB	✓

Client Build Manifests

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
_buildManifest.js gzip	457 B	457 B	✓
Overall change	457 B	457 B	✓

Rendered Page Sizes

	vercel/next.js canary	nkzawa/next.js middleware-no-node-buildin-module	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Commit: 5dfd5a4

timneutkens · 2022-04-15T14:26:57Z

packages/next/build/index.ts

@@ -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.'


@sokra is there a better way to exclude the compilation from showing these, an option maybe?

looks there is no other way as far as I checked https://github.com/webpack/webpack/blob/main/lib/ModuleNotFoundError.js#L63

timneutkens

@nkzawa this is only the starting point right? Next step being improving the error shown when you import these in the edge runtime? If so the change looks good and it can be landed.

nkzawa · 2022-04-18T06:49:58Z

@timneutkens yes, we're going to improve error messages as well

nkzawa · 2022-04-18T14:22:18Z

Seems this causes to break edge runtime. We need to fix that too.

nkzawa · 2022-04-19T08:42:59Z

Changed to apply the fix only to middleware for now, because of the above problem

ijjk · 2022-04-20T07:03:44Z

Failing test suites

Commit: 526a795

yarn testheadless test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts

middleware environment variables in node server reflect the usage inference > limits process.env to only contain env vars that are inferred from usage

Expand output

● middleware environment variables in node server reflect the usage inference › limits process.env to only contain env vars that are inferred from usage

expect(received).not.toThrow()

Error name:    "SyntaxError"
Error message: "Unexpected token < in JSON at position 0"

      38 |     let parsed: any
      39 |     expect(() => {
    > 40 |       parsed = JSON.parse(html)
         |                     ^
      41 |     }).not.toThrow()
      42 |     expect(parsed).toEqual({
      43 |       canBeInferred: 'can-be-inferred',

  at production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts:40:21
  at Object.<anonymous> (../node_modules/expect/build/toThrowMatchers.js:83:11)
  at Object.toThrow (../node_modules/expect/build/index.js:382:21)
  at Object.<anonymous> (production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts:41:12)
  at Object.<anonymous> (production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts:41:12)

Read more about building and testing Next.js in contributing.md.

x-ref: #36190 x-ref: #31506 * Move nodejs ptah module usage to next-server, keep base-server and web-server headless for `'path'` * Use a native module `path` for nodejs runtime and `path` polyfill for edge runtime

This reverts commit 94db083.

Remove the webpack breaking change message per error since it can be contained in all errors and the current implementation causes to truncate other error messages Follow up for #36190 ## Bug - [x] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md`

do not polyfill node built-in modules

c296a68

nkzawa requested review from timneutkens, ijjk, shuding, padmaia and huozhi as code owners April 15, 2022 13:41

ijjk added the type: next label Apr 15, 2022

Merge branch 'canary' into middleware-no-node-buildin-module

5ae8270

Merge branch 'canary' into middleware-no-node-buildin-module

5ffd627

timneutkens reviewed Apr 15, 2022

View reviewed changes

timneutkens previously approved these changes Apr 15, 2022

View reviewed changes

Merge branch 'canary' into middleware-no-node-buildin-module

b5246bb

Merge branch 'canary' into middleware-no-node-buildin-module

d113681

huozhi mentioned this pull request Apr 18, 2022

refactor: move fs based inc cache to next-server #36246

Closed

nkzawa added 2 commits April 19, 2022 15:29

Merge branch 'canary' into middleware-no-node-buildin-module

817463f

polyfill built-in modules for edge runtime for now

94db083

nkzawa dismissed timneutkens’s stale review via 94db083 April 19, 2022 08:42

Merge branch 'canary' into middleware-no-node-buildin-module

cd08116

huozhi mentioned this pull request Apr 20, 2022

Use fallbackable path module for node and edge runtime #36306

Merged

Merge branch 'canary' into middleware-no-node-buildin-module

dd23be0

nkzawa added 4 commits April 22, 2022 14:35

Merge branch 'canary' into middleware-no-node-buildin-module

bb20bfb

Merge branch 'canary' into middleware-no-node-buildin-module

4bd888b

Revert "polyfill built-in modules for edge runtime for now"

92146aa

This reverts commit 94db083.

Merge branch 'canary' into middleware-no-node-buildin-module

8ea6672

huozhi previously approved these changes Apr 22, 2022

View reviewed changes

update internal path import

0ffa323

huozhi dismissed their stale review via 0ffa323 April 22, 2022 13:28

nkzawa added 7 commits April 22, 2022 21:06

remove use of process polyfill

1c91e94

fix for lint

8127b1b

fix for lint

7cd01cf

fix a test

0fdb159

Merge branch 'canary' into middleware-no-node-buildin-module

39e149d

disable fallback check for process on edge runtime

526a795

set existing polyfill instead

5dfd5a4

shuding approved these changes Apr 25, 2022

View reviewed changes

kodiakhq bot merged commit a628256 into vercel:canary Apr 25, 2022

This was referenced Apr 25, 2022

Fix next node buildin module error message for edge runtime #36434

Merged

Fix to remove webpack message per error #36452

Merged

nkzawa deleted the middleware-no-node-buildin-module branch April 27, 2022 15:53

balazsorban44 mentioned this pull request May 12, 2022

Build fails to compile - module not found: can't resolve "child_process" 12.1.6 #36776

Closed

1 task

github-actions bot locked as resolved and limited conversation to collaborators May 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not polyfill node built-in modules on edge functions #36190

Do not polyfill node built-in modules on edge functions #36190

nkzawa commented Apr 15, 2022 •

edited

ijjk commented Apr 15, 2022 •

edited

timneutkens Apr 15, 2022

nkzawa Apr 22, 2022

timneutkens left a comment

nkzawa commented Apr 18, 2022

nkzawa commented Apr 18, 2022

nkzawa commented Apr 19, 2022

ijjk commented Apr 20, 2022 •

edited

Do not polyfill node built-in modules on edge functions #36190

Do not polyfill node built-in modules on edge functions #36190

Conversation

nkzawa commented Apr 15, 2022 • edited

Bug

Feature

Documentation / Examples

ijjk commented Apr 15, 2022 • edited

Stats from current PR

timneutkens Apr 15, 2022

Choose a reason for hiding this comment

nkzawa Apr 22, 2022

Choose a reason for hiding this comment

timneutkens left a comment

Choose a reason for hiding this comment

nkzawa commented Apr 18, 2022

nkzawa commented Apr 18, 2022

nkzawa commented Apr 19, 2022

ijjk commented Apr 20, 2022 • edited

Failing test suites

nkzawa commented Apr 15, 2022 •

edited

ijjk commented Apr 15, 2022 •

edited

ijjk commented Apr 20, 2022 •

edited