fix: revert #6340, definePlugin tests, warning box (#7174)

Co-authored-by: patak-dev <matias.capeletto@gmail.com>

docs/config/index.md

@@ -139,9 +139,11 @@ export default defineConfig(async ({ command, mode }) => {
 
   - Replacements are performed only when the match is surrounded by word boundaries (`\b`).
 
+  ::: warning
   Because it's implemented as straightforward text replacements without any syntax analysis, we recommend using `define` for CONSTANTS only.
 
   For example, `process.env.FOO` and `__APP_VERSION__` are good fits. But `process` or `global` should not be put into this option. Variables can be shimmed or polyfilled instead.
+  :::
 
   ::: tip NOTE
   For TypeScript users, make sure to add the type declarations in the `env.d.ts` or `vite-env.d.ts` file to get type checks and Intellisense.

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

@@ -20,13 +20,4 @@ test('string', async () => {
   expect(await page.textContent('.spread-array')).toBe(
     JSON.stringify([...defines.__STRING__])
   )
-  expect(await page.textContent('.import-file')).not.toBe(
-    `import * from "${defines.__IMPORT_FILE_NAME__}"`
-  )
-  expect(await page.textContent('.export-file')).not.toBe(
-    `export * from "${defines.__EXPORT_FILE_NAME__}"`
-  )
-  expect(await page.textContent('.path')).not.toBe(
-    `import * from "xxxx/${defines.PATH}"`
-  )
 })

packages/playground/define/index.html

@@ -9,9 +9,6 @@ <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>import file: <code class="import-file"></code></p>
-<p>export file: <code class="export-file"></code></p>
-<p>path: <code class="path"></code></p>
 
 <script type="module">
   const __VAR_NAME__ = true // ensure define doesn't replace var name
@@ -29,9 +26,6 @@ <h1>Define</h1>
     })
   )
   text('.spread-array', JSON.stringify([...`"${__STRING__}"`]))
-  text('.import-file', `import * from "__IMPORT_FILE_NAME__"`)
-  text('.export-file', `export * from "__EXPORT_FILE_NAME__"`)
-  text('.path', `import * from "xxxx/PATH"`)
 
   function text(el, text) {
     document.querySelector(el).textContent = text

packages/playground/define/vite.config.js

@@ -16,9 +16,6 @@ module.exports = {
       }
     },
     __VAR_NAME__: false,
-    'process.env.SOMEVAR': '"SOMEVAR"',
-    __IMPORT_FILE_NAME__: '"importFileName"',
-    __EXPORT_FILE_NAME__: '"exportFileName"',
-    PATH: '"filePath"'
+    'process.env.SOMEVAR': '"SOMEVAR"'
   }
 }

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

@@ -0,0 +1,39 @@
+import { definePlugin } from '../../plugins/define'
+import { resolveConfig } from '../../config'
+
+async function createDefinePluginTransform(
+  define: Record<string, any> = {},
+  build = true,
+  ssr = false
+) {
+  const config = await resolveConfig({ define }, build ? 'build' : 'serve')
+  const instance = definePlugin(config)
+  return async (code: string) => {
+    const result = await instance.transform.call({}, code, 'foo.ts', { ssr })
+    return result?.code || result
+  }
+}
+
+describe('definePlugin', () => {
+  test('replaces custom define', async () => {
+    const transform = await createDefinePluginTransform({
+      __APP_VERSION__: JSON.stringify('1.0')
+    })
+    expect(await transform('const version = __APP_VERSION__ ;')).toBe(
+      'const version = "1.0" ;'
+    )
+    expect(await transform('const version = __APP_VERSION__;')).toBe(
+      'const version = "1.0";'
+    )
+  })
+
+  test('replaces import.meta.env.SSR with false', async () => {
+    const transform = await createDefinePluginTransform()
+    expect(await transform('const isSSR = import.meta.env.SSR ;')).toBe(
+      'const isSSR = false ;'
+    )
+    expect(await transform('const isSSR = import.meta.env.SSR;')).toBe(
+      'const isSSR = false;'
+    )
+  })
+})

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

@@ -53,6 +53,7 @@ export function definePlugin(config: ResolvedConfig): Plugin {
         'globalThis.process.env.': `({}).`
       })
     }
+
     const replacements: Record<string, string> = {
       ...(isNeedProcessEnv ? processNodeEnv : {}),
       ...userDefine,
@@ -61,30 +62,20 @@ export function definePlugin(config: ResolvedConfig): Plugin {
     }
 
     const replacementsKeys = Object.keys(replacements)
-
-    if (!replacementsKeys.length) {
-      return [replacements, null]
-    }
-
-    const replacementsStr = replacementsKeys
-      .map((str) => {
-        return str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&')
-      })
-      .join('|')
-
-    // The following characters are not allowed because they are String boundaries
-    const characters = '[\'"`\\/-_]'
-
-    const pattern = new RegExp(
-      `(?<!${characters})` +
-        // Do not allow preceding '.', but do allow preceding '...' for spread operations
-        '(?<!(?<!\\.\\.)\\.)' +
-        `\\b(${replacementsStr})\\b` +
-        `(?!${characters})` +
-        // prevent trailing assignments
-        '(?!\\s*?=[^=])',
-      'g'
-    )
+    const pattern = replacementsKeys.length
+      ? new RegExp(
+          // Do not allow preceding '.', but do allow preceding '...' for spread operations
+          '(?<!(?<!\\.\\.)\\.)\\b(' +
+            replacementsKeys
+              .map((str) => {
+                return str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&')
+              })
+              .join('|') +
+            // prevent trailing assignments
+            ')\\b(?!\\s*?=[^=])',
+          'g'
+        )
+      : null
 
     return [replacements, pattern]
   }