Add warning for non-standard NODE_ENV value (vercel#12361)

Author: ijjk

errors/non-standard-node-env.md

@@ -0,0 +1,17 @@
+# Non-Standard Node_env
+
+#### Why This Error Occurred
+
+In your environment you set a non-standard value for `NODE_ENV`.
+
+Next.js automatically sets this environment value for you and also forces the correct value during bundling to ensure the bundles are optimized and code can be tree-shaken correctly.
+
+When you set a non-standard environment value like `staging` this causes inconsistent behavior since we override the value to the standard one during bundling e.g. `production` or `development`.
+
+#### Possible Ways to Fix It
+
+Remove any custom `NODE_ENV` environment variables and let Next.js automatically set the correct value for you.
+
+### Useful Links
+
+- [Environment Variables](https://en.wikipedia.org/wiki/Environment_variable)

packages/next/bin/next.ts

@@ -1,5 +1,7 @@
 #!/usr/bin/env node
+import * as log from '../build/output/log'
 import arg from 'next/dist/compiled/arg/index.js'
+import { NON_STANDARD_NODE_ENV } from '../lib/constants'
 ;['react', 'react-dom'].forEach(dependency => {
   try {
     // When 'npm link' is used it checks the clone location. Not the project.
@@ -85,6 +87,13 @@ if (args['--help']) {
 }
 
 const defaultEnv = command === 'dev' ? 'development' : 'production'
+
+const standardEnv = ['production', 'development', 'test']
+
+if (process.env.NODE_ENV && !standardEnv.includes(process.env.NODE_ENV)) {
+  log.warn(NON_STANDARD_NODE_ENV)
+}
+
 ;(process.env as any).NODE_ENV = process.env.NODE_ENV || defaultEnv
 
 // this needs to come after we set the correct NODE_ENV or

packages/next/lib/constants.ts

@@ -41,3 +41,5 @@ export const UNSTABLE_REVALIDATE_RENAME_ERROR =
   'https://nextjs.link/issg'
 
 export const GSSP_COMPONENT_MEMBER_ERROR = `can not be attached to a page's component and must be exported from the page. See more info here: https://err.sh/next.js/gssp-component-member`
+
+export const NON_STANDARD_NODE_ENV = `You are using a non-standard "NODE_ENV" value in your environment. This creates inconsistencies in the project and is strongly advised against. https://err.sh/next.js/non-standard-node-env`

packages/next/server/next.ts

@@ -1,4 +1,6 @@
 import Server, { ServerConstructor } from '../next-server/server/next-server'
+import { NON_STANDARD_NODE_ENV } from '../lib/constants'
+import * as log from '../build/output/log'
 
 type NextServerConstructor = Omit<ServerConstructor, 'staticMarkup'> & {
   /**
@@ -9,6 +11,16 @@ type NextServerConstructor = Omit<ServerConstructor, 'staticMarkup'> & {
 
 // This file is used for when users run `require('next')`
 function createServer(options: NextServerConstructor): Server {
+  const standardEnv = ['production', 'development', 'test']
+
+  if (
+    !(options as any).isNextDevCommand &&
+    process.env.NODE_ENV &&
+    !standardEnv.includes(process.env.NODE_ENV)
+  ) {
+    log.warn(NON_STANDARD_NODE_ENV)
+  }
+
   if (options.dev) {
     const Server = require('./next-dev-server').default
     return new Server(options)

test/integration/non-standard-node-env-warning/pages/index.js

@@ -0,0 +1 @@
+export default () => 'hi'

test/integration/non-standard-node-env-warning/server.js

@@ -0,0 +1,23 @@
+const http = require('http')
+const next = require('next')
+
+const dev = process.env.NODE_ENV !== 'production'
+const dir = __dirname
+const port = process.env.PORT || 3000
+
+const app = next({ dev, dir })
+const handleNextRequests = app.getRequestHandler()
+
+app.prepare().then(() => {
+  const server = new http.Server(async (req, res) => {
+    handleNextRequests(req, res)
+  })
+
+  server.listen(port, err => {
+    if (err) {
+      throw err
+    }
+
+    console.log(`> Ready on http://localhost:${port}`)
+  })
+})

test/integration/non-standard-node-env-warning/test/index.test.js

@@ -0,0 +1,115 @@
+/* eslint-env jest */
+/* global jasmine */
+import { join } from 'path'
+import {
+  findPort,
+  launchApp,
+  killApp,
+  waitFor,
+  initNextServerScript,
+} from 'next-test-utils'
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
+
+const appDir = join(__dirname, '..')
+const warningText = `You are using a non-standard "NODE_ENV" value in your environment`
+
+let appPort
+let app
+
+const startServer = async (optEnv = {}, opts) => {
+  const scriptPath = join(appDir, 'server.js')
+  appPort = appPort = await findPort()
+  const env = Object.assign({}, process.env, { PORT: `${appPort}` }, optEnv)
+
+  return initNextServerScript(
+    scriptPath,
+    /ready on/i,
+    env,
+    /ReferenceError: options is not defined/,
+    opts
+  )
+}
+
+describe('Non-Standard NODE_ENV', () => {
+  it('should not show the warning with no NODE_ENV set', async () => {
+    let output = ''
+
+    app = await launchApp(appDir, await findPort(), {
+      onStdout(msg) {
+        output += msg || ''
+      },
+    })
+    await waitFor(2000)
+    await killApp(app)
+    expect(output).not.toContain(warningText)
+  })
+
+  it('should not show the warning with NODE_ENV set to valid value', async () => {
+    let output = ''
+
+    app = await launchApp(appDir, await findPort(), {
+      env: {
+        NODE_ENV: 'development',
+      },
+      onStdout(msg) {
+        output += msg || ''
+      },
+    })
+    await waitFor(2000)
+    await killApp(app)
+    expect(output).not.toContain(warningText)
+  })
+
+  it('should not show the warning with NODE_ENV set to valid value (custom server)', async () => {
+    let output = ''
+
+    app = await startServer(
+      {
+        NODE_ENV: 'development',
+      },
+      {
+        onStdout(msg) {
+          output += msg || ''
+        },
+      }
+    )
+    await waitFor(2000)
+    await killApp(app)
+    expect(output).not.toContain(warningText)
+  })
+
+  it('should show the warning with NODE_ENV set to invalid value', async () => {
+    let output = ''
+
+    app = await launchApp(appDir, await findPort(), {
+      env: {
+        NODE_ENV: 'abc',
+      },
+      onStdout(msg) {
+        output += msg || ''
+      },
+    })
+    await waitFor(2000)
+    await killApp(app)
+    expect(output).toContain(warningText)
+  })
+
+  it('should show the warning with NODE_ENV set to invalid value (custom server)', async () => {
+    let output = ''
+
+    app = await startServer(
+      {
+        NODE_ENV: 'abc',
+      },
+      {
+        onStdout(msg) {
+          output += msg || ''
+        },
+      }
+    )
+    await waitFor(2000)
+    await killApp(app)
+    expect(output).toContain(warningText)
+  })
+})