"process.env.NODE_ENV" causes invalid code injection

[tony19]

Describe the bug

Vite's client-inject plugin indiscriminately replaces process.env.NODE_ENV, even in comments and string literals. This is similar to #9790, where the issue happens in Vite's define plugin.

This becomes a problem in the following Vue template:

<template>
  <!-- NODE_ENV is process.env.NODE_ENV -->
  <h2>demo</h2>
</template>

@vue/compiler-sfc transforms that into this:

_createCommentVNode(" NODE_ENV is process.env.NODE_ENV ")
⋮

...which is passed to the client-inject plugin , which transforms that into this invalid JavaScript:

_createCommentVNode(" NODE_ENV is "development" ")
⋮

...which results in this runtime error:

Uncaught SyntaxError: missing ) after argument list

Also see #3304 (comment)

Reproduction

https://stackblitz.com/edit/vitejs-vite-yw5t84?file=src%2Fcomponents%2FHelloWorld.vue

System Info

System:
    OS: macOS 11.6.7
    CPU: (8) x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
    Memory: 629.66 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.17.0 - ~/n/bin/node
    Yarn: 1.22.19 - ~/n/bin/yarn
    npm: 8.15.0 - ~/n/bin/npm
  Browsers:
    Chrome: 104.0.5112.101
    Chrome Canary: 107.0.5259.0
    Firefox: 103.0.2
    Safari: 15.6
  npmPackages:
    vite: workspace:* => 3.0.9 

Used Package Manager

npm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[tony19]

Interestingly, when running unit tests that trigger this bug (e.g., "process.env.NODE_ENV" in a Vue template in the playground), the page APIs (i.e., page.textContent(), etc.) continue waiting even though the test app has crashed in the browser. It would be nicer for Vitest/Playwright to somehow detect that and fail immediately with a helpful error message.

[bluwy]

Good point. If we merge #9791, it does mean that this bug wouldn't happen as well, but I don't see another way around this, except doing the replacement in define plugin instead, which means we would have to run the define plugin for dev too, maybe only for process.env.NODE_ENV.

[tony19]

I don't think the other PR would prevent this bug from occurring. This plugin still does a string replacement regardless of what the define plugin does. Am I missing something?

Oh, I think you mean that the define plugin would replace the process.env.NODE_ENV before it gets here, which makes me wonder why the replacement is needed again here at all.

[sharh]

In the define.ts, If not in lib mode, will set process.env.NODE_ENV. It may be because many third-party libraries rely on this environment variable. You change your environment variable name to resolve this problem.

[peterboyer]

Just ran into this problem too.

My workaround/hack was to substitute using single-quotes instead of JSON stringified double-quotes as to not break mistakenly identified occurances (inside double-quoted string literals) and also work as a valid string literal on its own (for intended replacements).

// vite.config.ts
{
  define: {
    "process.env.NODE_ENV": `'${process.env.NODE_ENV}'`
  }
}