fix: nested comments and strings, new regexp utils

CONTRIBUTING.md

@@ -79,7 +79,7 @@ Each test can be run under either dev server mode or build mode.
 
 - `pnpm run test-build` runs tests only under build mode.
 
-- You can also use `pnpm run test-serve -- [match]` or `pnpm run test-build -- [match]` to run tests in a specific playground package, e.g. `pnpm run test-serve -- css` will run tests for both `playground/css` and `playground/css-codesplit` under serve mode.
+- You can also use `pnpm run test-serve -- [match]` or `pnpm run test-build -- [match]` to run tests in a specific playground package, e.g. `pnpm run test-serve -- asset` will run tests for both `playground/asset` and `vite/src/node/__tests__/asset` under serve mode and `vite/src/node/__tests__/**/*` just run in serve mode.
 
   Note package matching is not available for the `pnpm test` script, which always runs all tests.
 

packages/vite/src/node/__tests__/cleanString.spec.ts

@@ -0,0 +1,108 @@
+import { emptyString } from '../../node/cleanString'
+
+test('comments', () => {
+  expect(
+    emptyString(`
+    // comment1 // comment
+    // comment1
+    /* coment2 */
+    /*
+      // coment3
+    */
+    /* // coment3 */
+    /* // coment3 */ // comment
+    // comment 4 /* comment 5 */
+  `).trim()
+  ).toBe('')
+})
+
+test('strings', () => {
+  const clean = emptyString(`
+    // comment1
+    const a = 'aaaa'
+    /* coment2 */
+    const b = "bbbb"
+    /*
+      // coment3
+    */
+    /* // coment3 */
+    // comment 4 /* comment 5 */
+  `)
+  expect(clean).toMatch("const a = '\0\0\0\0'")
+  expect(clean).toMatch('const b = "\0\0\0\0"')
+})
+
+test('strings comment nested', () => {
+  expect(
+    emptyString(`
+    // comment 1 /* " */
+    const a = "a //"
+    // comment 2 /* " */
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    // comment 1 /* ' */
+    const a = "a //"
+    // comment 2 /* ' */
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    // comment 1 /* \` */
+    const a = "a //"
+    // comment 2 /* \` */
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a //"
+    console.log("console")
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a /*"
+    console.log("console")
+    const b = "b */"
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a ' "
+    console.log("console")
+    const b = "b ' "
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a \` "
+    console.log("console")
+    const b = "b \` "
+  `)
+  ).toMatch('const a = "\0\0\0\0"')
+})
+
+test('find empty string flag in raw index', () => {
+  const str = `
+    const a = "aaaaa"
+    const b = "bbbbb"
+  `
+  const clean = emptyString(str)
+  expect(clean).toMatch('const a = "\0\0\0\0\0"')
+  expect(clean).toMatch('const b = "\0\0\0\0\0"')
+
+  const aIndex = str.indexOf('const a = "aaaaa"')
+  const aStart = clean.indexOf('\0\0\0\0\0', aIndex)
+  expect(str.slice(aStart, aStart + 5)).toMatch('aaaaa')
+
+  const bIndex = str.indexOf('const b = "bbbbb"')
+  const bStart = clean.indexOf('\0\0\0\0\0', bIndex)
+  expect(str.slice(bStart, bStart + 5)).toMatch('bbbbb')
+})

packages/vite/src/node/cleanString.ts

@@ -0,0 +1,14 @@
+// bank on the non-overlapping nature of regex matches and combine all filters into one giant regex
+// /`([^`\$\{\}]|\$\{(`|\g<1>)*\})*`/g can match nested string template
+// but js not support match expression(\g<0>). so clean string template(`...`) in other ways.
+const cleanerRE = /"[^"]*"|'[^']*'|\/\*(.|[\r\n])*?\*\/|\/\/.*/g
+
+const blankReplacer = (s: string) => ' '.repeat(s.length)
+const stringBlankReplacer = (s: string) =>
+  `${s[0]}${'\0'.repeat(s.length - 2)}${s[0]}`
+
+export function emptyString(raw: string): string {
+  return raw.replace(cleanerRE, (s: string) =>
+    s[0] === '/' ? blankReplacer(s) : stringBlankReplacer(s)
+  )
+}

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

@@ -3,12 +3,7 @@ import MagicString from 'magic-string'
 import path from 'path'
 import { fileToUrl } from './asset'
 import type { ResolvedConfig } from '../config'
-import {
-  multilineCommentsRE,
-  singlelineCommentsRE,
-  stringsRE,
-  blankReplacer
-} from '../utils'
+import { emptyString } from '../cleanString'
 
 /**
  * Convert `new URL('./foo.png', import.meta.url)` to its resolved built URL
@@ -29,19 +24,16 @@ export function assetImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
         code.includes('new URL') &&
         code.includes(`import.meta.url`)
       ) {
-        const importMetaUrlRE =
+        let s: MagicString | undefined
+        const assetImportMetaUrlRE =
           /\bnew\s+URL\s*\(\s*('[^']+'|"[^"]+"|`[^`]+`)\s*,\s*import\.meta\.url\s*,?\s*\)/g
-        const noCommentsCode = code
-          .replace(multilineCommentsRE, blankReplacer)
-          .replace(singlelineCommentsRE, blankReplacer)
-          .replace(stringsRE, (m) => `'${'\0'.repeat(m.length - 2)}'`)
+        const cleanString = emptyString(code)
 
-        let s: MagicString | null = null
         let match: RegExpExecArray | null
-        while ((match = importMetaUrlRE.exec(noCommentsCode))) {
+        while ((match = assetImportMetaUrlRE.exec(cleanString))) {
           const { 0: exp, 1: emptyUrl, index } = match
 
-          const urlStart = exp.indexOf(emptyUrl) + index
+          const urlStart = cleanString.indexOf(emptyUrl, index)
           const urlEnd = urlStart + emptyUrl.length
           const rawUrl = code.slice(urlStart, urlEnd)
 
@@ -74,8 +66,9 @@ export function assetImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
           // Get final asset URL. Catch error if the file does not exist,
           // in which we can resort to the initial URL and let it resolve in runtime
           const builtUrl = await fileToUrl(file, config, this).catch(() => {
+            const rawExp = code.slice(index, index + exp.length)
             config.logger.warnOnce(
-              `\n${exp} doesn't exist at build time, it will remain unchanged to be resolved at runtime`
+              `\n${rawExp} doesn't exist at build time, it will remain unchanged to be resolved at runtime`
             )
             return url
           })
@@ -92,8 +85,8 @@ export function assetImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
             map: config.build.sourcemap ? s.generateMap({ hires: true }) : null
           }
         }
+        return null
       }
-      return null
     }
   }
 }

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

@@ -2,65 +2,56 @@ import JSON5 from 'json5'
 import type { ResolvedConfig } from '../config'
 import type { Plugin } from '../plugin'
 import { fileToUrl } from './asset'
-import {
-  blankReplacer,
-  cleanUrl,
-  injectQuery,
-  multilineCommentsRE,
-  singlelineCommentsRE,
-  stringsRE
-} from '../utils'
+import { cleanUrl, injectQuery } from '../utils'
 import path from 'path'
 import { workerFileToUrl } from './worker'
 import { parseRequest } from '../utils'
 import { ENV_ENTRY, ENV_PUBLIC_PATH } from '../constants'
 import MagicString from 'magic-string'
 import type { ViteDevServer } from '..'
 import type { RollupError } from 'rollup'
+import { emptyString } from '../cleanString'
 
 type WorkerType = 'classic' | 'module' | 'ignore'
+const ignoreFlagRE = /\/\*\s*@vite-ignore\s*\*\//
 
 const WORKER_FILE_ID = 'worker_url_file'
 
-function getWorkerType(
-  code: string,
-  noCommentsCode: string,
-  i: number
-): WorkerType {
+function getWorkerType(raw: string, clean: string, i: number): WorkerType {
   function err(e: string, pos: number) {
     const error = new Error(e) as RollupError
     error.pos = pos
     throw error
   }
 
-  const commaIndex = noCommentsCode.indexOf(',', i)
+  const commaIndex = clean.indexOf(',', i)
   if (commaIndex === -1) {
     return 'classic'
   }
-  const endIndex = noCommentsCode.indexOf(')', i)
+  const endIndex = clean.indexOf(')', i)
 
   // case: ') ... ,' mean no worker options params
   if (commaIndex > endIndex) {
     return 'classic'
   }
 
   // need to find in comment code
-  let workerOptsString = code.substring(commaIndex + 1, endIndex)
+  const workerOptString = raw.substring(commaIndex + 1, endIndex)
 
-  const hasViteIgnore = /\/\*\s*@vite-ignore\s*\*\//.test(workerOptsString)
+  const hasViteIgnore = ignoreFlagRE.test(workerOptString)
   if (hasViteIgnore) {
     return 'ignore'
   }
 
   // need to find in no comment code
-  workerOptsString = noCommentsCode.substring(commaIndex + 1, endIndex)
-  if (!workerOptsString.trim().length) {
+  const cleanWorkerOptString = clean.substring(commaIndex + 1, endIndex)
+  if (!cleanWorkerOptString.trim().length) {
     return 'classic'
   }
 
   let workerOpts: { type: WorkerType } = { type: 'classic' }
   try {
-    workerOpts = JSON5.parse(workerOptsString)
+    workerOpts = JSON5.parse(workerOptString)
   } catch (e) {
     // can't parse by JSON5, so the worker options had unexpect char.
     err(
@@ -113,28 +104,22 @@ export function workerImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
           code: injectEnv + code
         }
       }
+      let s: MagicString | undefined
       if (
         (code.includes('new Worker') || code.includes('new ShareWorker')) &&
         code.includes('new URL') &&
         code.includes(`import.meta.url`)
       ) {
-        const importMetaUrlRE =
+        const cleanString = emptyString(code)
+        const workerImportMetaUrlRE =
           /\bnew\s+(Worker|SharedWorker)\s*\(\s*(new\s+URL\s*\(\s*('[^']+'|"[^"]+"|`[^`]+`)\s*,\s*import\.meta\.url\s*\))/g
-        const noCommentsCode = code
-          .replace(multilineCommentsRE, blankReplacer)
-          .replace(singlelineCommentsRE, blankReplacer)
-
-        const noStringCode = noCommentsCode.replace(
-          stringsRE,
-          (m) => `'${' '.repeat(m.length - 2)}'`
-        )
+
         let match: RegExpExecArray | null
-        let s: MagicString | null = null
-        while ((match = importMetaUrlRE.exec(noStringCode))) {
+        while ((match = workerImportMetaUrlRE.exec(cleanString))) {
           const { 0: allExp, 2: exp, 3: emptyUrl, index } = match
           const urlIndex = allExp.indexOf(exp) + index
 
-          const urlStart = allExp.indexOf(emptyUrl) + index
+          const urlStart = cleanString.indexOf(emptyUrl, index)
           const urlEnd = urlStart + emptyUrl.length
           const rawUrl = code.slice(urlStart, urlEnd)
 
@@ -156,7 +141,7 @@ export function workerImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
           s ||= new MagicString(code)
           const workerType = getWorkerType(
             code,
-            noCommentsCode,
+            cleanString,
             index + allExp.length
           )
           const file = path.resolve(path.dirname(id), rawUrl.slice(1, -1))
@@ -172,14 +157,15 @@ export function workerImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
             contentOnly: true
           })
         }
-        if (s) {
-          return {
-            code: s.toString(),
-            map: config.build.sourcemap ? s.generateMap({ hires: true }) : null
-          }
+      }
+
+      if (s) {
+        return {
+          code: s.toString(),
+          map: config.build.sourcemap ? s.generateMap({ hires: true }) : null
         }
-        return null
       }
+      return null
     }
   }
 }

packages/vite/src/node/utils.ts

@@ -733,4 +733,3 @@ export function parseRequest(id: string): Record<string, string> | null {
 }
 
 export const blankReplacer = (match: string) => ' '.repeat(match.length)
-export const stringsRE = /"[^"]*"|'[^']*'|`[^`]*`/g