Fix global process testing for the process polyfill

[Schniz]

When there is a DOM element with id of process, the DOM marks it as a global, so window.process would exist. We should check for process.env to make sure it is available too.

[ijjk]

Failing test suites

Commit: 4822dda

test/integration/polyfills/test/index.test.js

• Polyfills > should contain generated page count in output

Expand output

● Polyfills › should contain generated page count in output

expect(received).toContain(expected) // indexOf

Expected substring: "Generating static pages (0/4)"
Received string:    "warn  - The Next.js plugin was not detected in your ESLint configuration. See https://nextjs.org/docs/basic-features/eslint#migrating-existing-config
info  - Checking validity of types...
info  - Creating an optimized production build...
info  - Compiled successfully
info  - Collecting page data...
info  - Generating static pages (0/5)
info  - Generating static pages (1/5)
info  - Generating static pages (2/5)
info  - Generating static pages (3/5)
info  - Generating static pages (5/5)
info  - Finalizing page optimization...·
Page                                       Size     First Load JS
┌ ○ /                                      2.1 kB         73.4 kB
├ ○ /404                                   195 B          71.5 kB
├ ○ /fetch                                 844 B          72.1 kB
└ ○ /process                               981 B          72.3 kB
+ First Load JS shared by all              71.3 kB
  ├ chunks/framework-a3ba3e4fcbe915c4.js   42 kB
  ├ chunks/main-b7e09d2c432d9966.js        26.9 kB
  ├ chunks/pages/_app-26477814f4b13881.js  1.37 kB
  └ chunks/webpack-8d8b6780ba86e933.js     970 B·
○  (Static)  automatically rendered as static HTML (uses no initial props)·
"

  40 |     const browser = await webdriver(appPort, '/process')
  41 |     const text = await browser.elementByCss('#process').text()
> 42 |
     | ^
  43 |     expect(text).toBe('Hello, stranger')
  44 |
  45 |     await browser.close()

  at Object.<anonymous> (integration/polyfills/test/index.test.js:42:24)

[styfle]

I think this needs to mutate global.process if it exists because its referencing the DOM element so you just need to add process.env to it too. Something like this:

const polyfill = require('../../compiled/process')

module.exports = { 
  ...global.process,
  ...polyfill
}

[sokra]

I don't think that's a good idea. I think we should not use the global process object when it's not a "process" thing.

Otherwise you get random properties from the DOM objects into the process.

[Schniz]

Sounds like a production decision, I'm fine either way. I'm more biased towards the current implementation, because the following code:

// pages/my-page.js
import {useEffect} from 'react'
export default function MyComponent() {
  useEffect(() => {
    console.log(window.process.dataset, process.dataset)
  });
  return <div id="preview" data-hey="ho">Hello, {process.env.NEXT_PUBLIC_VISITOR}</div>
}

renders Hello, stranger and console logs DOMStringMap { hey: "ho" }, undefined. So we don't "leak" DOM data into what is treated as Node.js API when server rendering. The window. usage is explicit.

[ijjk]

This looks good to me as it handles the cases we identified specifically where an element with id of process exists we will use the polyfill since the element won't have a proper env field.

[ijjk]

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
buildDuration	14.7s	15.3s	⚠️ +608ms
buildDurationCached	3.2s	3.1s	-130ms
nodeModulesSize	354 MB	354 MB	⚠️ +426 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.873	2.936	⚠️ +0.06
/ avg req/sec	870.04	851.45	⚠️ -18.59
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.306	1.312	⚠️ +0.01
/error-in-render avg req/sec	1914.06	1905.27	⚠️ -8.79

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	27.2 kB	27.2 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71 kB	71 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
_app-HASH.js gzip	1.37 kB	1.37 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	326 B	326 B	✓
dynamic-HASH.js gzip	2.37 kB	2.37 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	919 B	919 B	✓
image-HASH.js gzip	4.7 kB	4.7 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.13 kB	2.13 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.1 kB	14.1 kB	✓

Client Build Manifests

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
index.html gzip	531 B	531 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

---

Default Build with SWC (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
buildDuration	15.8s	15.2s	-572ms
buildDurationCached	3.1s	3.1s	⚠️ +21ms
nodeModulesSize	354 MB	354 MB	⚠️ +426 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.902	3.509	⚠️ +0.61
/ avg req/sec	861.5	712.47	⚠️ -149.03
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.365	1.325	-0.04
/error-in-render avg req/sec	1832.09	1887.23	+55.14

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	27.3 kB	27.3 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.2 kB	71.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.36 kB	2.36 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	906 B	906 B	✓
image-HASH.js gzip	4.72 kB	4.72 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.19 kB	2.19 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14 kB	14 kB	✓

Client Build Manifests

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
_buildManifest.js gzip	458 B	458 B	✓
Overall change	458 B	458 B	✓

Rendered Page Sizes

	vercel/next.js canary	Schniz/next.js fix-global-process-testing	Change
index.html gzip	530 B	530 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Commit: 0956aa7