fix: make wrong process.env.NODE_ENV value a hard failure

vikejs · Jan 21, 2024 · f3926bd · f3926bd
1 parent 3af374d
commit f3926bd
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 18 deletions.
diff --git a/vike/node/plugin/index.ts b/vike/node/plugin/index.ts
@@ -35,18 +35,7 @@ import { envVarsPlugin } from './plugins/envVars.js'
 import pc from '@brillout/picocolors'
 import { fileEnv } from './plugins/fileEnv.js'
 
-assertUsage(
-  isNodeEnvDev() ||
-    // TODO: enable this assertion after the vike CLI is implemented
-    (true as boolean),
-
-  [
-    pc.cyan(`process.env.NODE_ENV === ${JSON.stringify(getNodeEnv())}`),
-    '(which Vike interprets as a non-development environment https://vike.dev/NODE_ENV)',
-    'while the vike/plugin module is loaded.',
-    vikeVitePluginLoadedInProductionError
-  ].join(' ')
-)
+assertNodeEnv()
 markEnvAsVikePluginLoaded()
 
 // Return as `any` to avoid Plugin type mismatches when there are multiple Vite versions installed
@@ -95,3 +84,22 @@ Object.defineProperty(plugin, 'apply', {
     )
   }
 })
+
+function assertNodeEnv() {
+  const nodeEnv = getNodeEnv()
+  if (nodeEnv === 'test') return
+  // We should change this to be a warning if it blocks users (e.g. if a bad-citizen tool sets a wrong process.env.NODE_ENV value).
+  assertUsage(
+    /* We can enable this assertion after Vike's CLI is implemented and using Vite's CLI is deprecated (we can then check whether the context is a `$ vike build`).
+    isNodeEnvDev() || isVikeCliBuild(),
+    /*/
+    isNodeEnvDev() || (true as boolean),
+    ///*/
+    [
+      pc.cyan(`process.env.NODE_ENV === ${JSON.stringify(nodeEnv)}`),
+      '(which Vike interprets as a non-development environment https://vike.dev/NODE_ENV)',
+      'while the vike/plugin module is loaded.',
+      vikeVitePluginLoadedInProductionError
+    ].join(' ')
+  )
+}
diff --git a/vike/node/plugin/shared/isViteCliCall.ts b/vike/node/plugin/shared/isViteCliCall.ts
@@ -4,7 +4,7 @@ export { getViteConfigFromCli }
 import { assert, isObject, toPosixPath } from '../utils.js'
 import { cac } from 'cac'
 
-function isViteCliCall() {
+function isViteCliCall(): boolean {
   let execPath = process.argv[1]
   assert(execPath)
   execPath = toPosixPath(execPath)

diff --git a/vike/node/runtime/globalContext.ts b/vike/node/runtime/globalContext.ts
@@ -13,6 +13,7 @@ import {
   assertWarning,
   getGlobalObject,
   getNodeEnv,
+  isNodeEnvDev,
   isPlainObject,
   objectAssign
 } from './utils.js'
@@ -209,15 +210,15 @@ function assertViteManifest(manifest: unknown): asserts manifest is ViteManifest
 function assertNodeEnv(hasViteDevServer: boolean) {
   const nodeEnv = getNodeEnv()
   if (nodeEnv === null || nodeEnv === 'test') return
-  const isDevNodeEnv = [undefined, '', 'dev', 'development'].includes(nodeEnv)
-  // calling Vite's createServer() is enough for hasViteDevServer to be true, even without actually adding Vite's development middleware to the server: https://github.com/vikejs/vike/issues/792#issuecomment-1516830759
-  assertWarning(
+  const isDevNodeEnv = isNodeEnvDev()
+  // - Calling Vite's createServer() is enough for hasViteDevServer to be true, even without actually adding Vite's development middleware to the server: https://github.com/vikejs/vike/issues/792#issuecomment-1516830759
+  // - We should change this to be a warning if it blocks users (e.g. if a bad-citizen tool sets a wrong process.env.NODE_ENV value).
+  assertUsage(
     hasViteDevServer === isDevNodeEnv,
     `Vite's development server was${hasViteDevServer ? '' : "n't"} instantiated while the environment is set to be a ${
       isDevNodeEnv ? 'development' : 'production'
     } environment by ${pc.cyan(
       `process.env.NODE_ENV === ${JSON.stringify(nodeEnv)}`
-    )} which is contradictory, see https://vike.dev/NODE_ENV`,
-    { onlyOnce: true }
+    )} which is contradictory, see https://vike.dev/NODE_ENV`
   )
 }