feat: allow any JS identifier in define, not ASCII-only (#5972)

Author: jonaskuske

docs/config/index.md

@@ -164,7 +164,7 @@ export default defineConfig(({ command, mode }) => {
 
   - To be consistent with [esbuild behavior](https://esbuild.github.io/api/#define), expressions must either be a JSON object (null, boolean, number, string, array, or object) or a single identifier.
 
-  - Replacements are performed only when the match is surrounded by word boundaries (`\b`).
+  - Replacements are performed only when the match isn't surrounded by other letters, numbers, `_` or `$`.
 
   ::: warning
   Because it's implemented as straightforward text replacements without any syntax analysis, we recommend using `define` for CONSTANTS only.

packages/playground/define/__tests__/define.spec.ts

@@ -20,6 +20,14 @@ test('string', async () => {
   expect(await page.textContent('.spread-array')).toBe(
     JSON.stringify([...defines.__STRING__])
   )
+  expect(await page.textContent('.dollar-identifier')).toBe(
+    String(defines.$DOLLAR)
+  )
+  expect(await page.textContent('.unicode-identifier')).toBe(
+    String(defines.ÖUNICODE_LETTERɵ)
+  )
+  expect(await page.textContent('.no-identifier-substring')).toBe(String(true))
+  expect(await page.textContent('.no-property')).toBe(String(true))
   // html would't need to define replacement
   expect(await page.textContent('.exp-define')).toBe('__EXP__')
   expect(await page.textContent('.import-json')).toBe('__EXP__')

packages/playground/define/index.html

@@ -9,6 +9,10 @@ <h1>Define</h1>
 <p>process as property: <code class="process-as-property"></code></p>
 <p>spread object: <code class="spread-object"></code></p>
 <p>spread array: <code class="spread-array"></code></p>
+<p>dollar identifier: <code class="dollar-identifier"></code></p>
+<p>unicode identifier: <code class="unicode-identifier"></code></p>
+<p>no property: <code class="no-property"></code></p>
+<p>no identifier substring: <span class="no-identifier-substring"></span></p>
 <p>define variable in html: <code class="exp-define">__EXP__</code></p>
 <p>import json: <code class="import-json"></code></p>
 
@@ -28,6 +32,15 @@ <h1>Define</h1>
     })
   )
   text('.spread-array', JSON.stringify([...`"${__STRING__}"`]))
+  text('.dollar-identifier', $DOLLAR)
+  text('.unicode-identifier', ÖUNICODE_LETTERɵ)
+
+  // make sure these kinds of use are NOT replaced:
+  const obj = { [`${'_'}_EXP__`]: true }
+  text('.no-property', obj.__EXP__)
+
+  window[`${'_'}_EXP__SUBSTR__`] = true
+  text('.no-identifier-substring', __EXP__SUBSTR__)
 
   import dataJson from './data.json'
   text('.import-json', dataJson.foo)

packages/playground/define/vite.config.js

@@ -15,7 +15,9 @@ module.exports = {
         }
       }
     },
-    __VAR_NAME__: false,
-    'process.env.SOMEVAR': '"SOMEVAR"'
+    'process.env.SOMEVAR': '"SOMEVAR"',
+    $DOLLAR: 456,
+    ÖUNICODE_LETTERɵ: 789,
+    __VAR_NAME__: false
   }
 }

packages/vite/src/node/plugins/define.ts

@@ -68,16 +68,18 @@ export function definePlugin(config: ResolvedConfig): Plugin {
     const replacementsKeys = Object.keys(replacements)
     const pattern = replacementsKeys.length
       ? new RegExp(
-          // Do not allow preceding '.', but do allow preceding '...' for spread operations
-          '(?<!(?<!\\.\\.)\\.)\\b(' +
+          // Mustn't be preceded by a char that can be part of an identifier
+          // or a '.' that isn't part of a spread operator
+          '(?<![\\p{L}\\p{N}_$]|(?<!\\.\\.)\\.)(' +
             replacementsKeys
               .map((str) => {
                 return str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&')
               })
               .join('|') +
-            // prevent trailing assignments
-            ')\\b(?!\\s*?=[^=])',
-          'g'
+            // Mustn't be followed by a char that can be part of an identifier
+            // or an assignment (but allow equality operators)
+            ')(?![\\p{L}\\p{N}_$]|\\s*?=[^=])',
+          'gu'
         )
       : null