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,115 @@
+import { emptyString, findEmptyStringRawIndex } from '../../node/cleanString'
+
+test('comments', () => {
+  expect(
+    emptyString(`
+    // comment1 // comment
+    // comment1
+    /* coment2 */
+    /*
+      // coment3
+    */
+    /* // coment3 */
+    /* // coment3 */ // comment
+    // comment 4 /* comment 5 */
+  `).clean.trim()
+  ).toBe('')
+})
+
+test('strings', () => {
+  const clean = emptyString(`
+    // comment1
+    const a = 'aaaa'
+    /* coment2 */
+    const b = "bbbb"
+    /*
+      // coment3
+    */
+    /* // coment3 */
+    // comment 4 /* comment 5 */
+  `)
+  expect(clean.clean).toMatch("const a = '\0\0\0\0'")
+  expect(clean.clean).toMatch('const b = "\0\0\0\0"')
+})
+
+test('strings comment nested', () => {
+  expect(
+    emptyString(`
+    // comment 1 /* " */
+    const a = "a //"
+    // comment 2 /* " */
+  `).clean
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    // comment 1 /* ' */
+    const a = "a //"
+    // comment 2 /* ' */
+  `).clean
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    // comment 1 /* \` */
+    const a = "a //"
+    // comment 2 /* \` */
+  `).clean
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a //"
+    console.log("console")
+  `).clean
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a /*"
+    console.log("console")
+    const b = "b */"
+  `).clean
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a ' "
+    console.log("console")
+    const b = "b ' "
+  `).clean
+  ).toMatch('const a = "\0\0\0\0"')
+
+  expect(
+    emptyString(`
+    const a = "a \` "
+    console.log("console")
+    const b = "b \` "
+  `).clean
+  ).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.clean).toMatch('const a = "\0\0\0\0\0"')
+  expect(clean.clean).toMatch('const b = "\0\0\0\0\0"')
+
+  const aIndex = str.indexOf('const a = "aaaaa"')
+  const a = findEmptyStringRawIndex(clean.clean, '\0\0\0\0\0', aIndex)
+  expect(str.slice(a[0], a[1])).toMatch('aaaaa')
+
+  const bIndex = str.indexOf('const b = "bbbbb"')
+  const b = findEmptyStringRawIndex(clean.clean, '\0\0\0\0\0', bIndex)
+  expect(str.slice(b[0], b[1])).toMatch('bbbbb')
+})
+
+// describe('template string nested', () => {
+//   const str = "`##${a + b + `##${c + `##${d}`}##`}##`"
+
+//   const clean = emptyString(str)
+//   expect(clean.clean).toMatch('`\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0`')
+// })

packages/vite/src/node/cleanString.ts

@@ -0,0 +1,53 @@
+// 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 interface CleanString {
+  clean: string
+  raw: string
+}
+
+function isCleanString(obj: any): obj is CleanString {
+  return obj.raw && obj.clean
+}
+
+export function emptyCommentsString(raw: string): CleanString {
+  const res: CleanString = {
+    raw: raw,
+    clean: raw.replace(cleanerRE, (s: string) =>
+      s[0] === '/' ? blankReplacer(s) : s
+    )
+  }
+  return res
+}
+
+export function emptyString(raw: string | CleanString): CleanString {
+  const res: CleanString = { raw: '', clean: '' }
+  if (isCleanString(raw)) {
+    res.raw = raw.raw
+    res.clean = raw.clean
+  } else {
+    res.raw = raw
+    res.clean = raw
+  }
+  res.clean = res.clean.replace(cleanerRE, (s: string) =>
+    s[0] === '/' ? blankReplacer(s) : stringBlankReplacer(s)
+  )
+  // TODO replace string template
+  return res
+}
+
+export function findEmptyStringRawIndex(
+  clean: string,
+  emptyFlag: string,
+  start: number
+): [number, number] {
+  const flagIndex = clean.indexOf(emptyFlag, start)
+  const flagEndIndex = flagIndex + emptyFlag.length
+  return [flagIndex, flagEndIndex]
+}

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, findEmptyStringRawIndex } from '../cleanString'
 
 /**
  * Convert `new URL('./foo.png', import.meta.url)` to its resolved built URL
@@ -25,72 +20,73 @@ export function assetImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
     name: 'vite:asset-import-meta-url',
     async transform(code, id, options) {
       if (
-        !options?.ssr &&
-        code.includes('new URL') &&
-        code.includes(`import.meta.url`)
+        options?.ssr ||
+        !code.includes('new URL') ||
+        !code.includes(`import.meta.url`)
       ) {
-        const importMetaUrlRE =
-          /\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)}'`)
-
-        let s: MagicString | null = null
-        let match: RegExpExecArray | null
-        while ((match = importMetaUrlRE.exec(noCommentsCode))) {
-          const { 0: exp, 1: emptyUrl, index } = match
+        return null
+      }
+      let s: MagicString | undefined
+      let match: RegExpExecArray | null
+      const assetImportMetaUrlRE =
+        /\bnew\s+URL\s*\(\s*('[^']+'|"[^"]+"|`[^`]+`)\s*,\s*import\.meta\.url\s*,?\s*\)/g
+      const cleanString = emptyString(code)
+      while ((match = assetImportMetaUrlRE.exec(cleanString.clean))) {
+        const { 0: exp, 1: emptyUrl, index } = match
 
-          const urlStart = exp.indexOf(emptyUrl) + index
-          const urlEnd = urlStart + emptyUrl.length
-          const rawUrl = code.slice(urlStart, urlEnd)
+        const [urlStart, urlEnd] = findEmptyStringRawIndex(
+          cleanString.clean,
+          emptyUrl,
+          index
+        )
+        const rawUrl = code.slice(urlStart, urlEnd)
 
-          if (!s) s = new MagicString(code)
+        if (!s) s = new MagicString(code)
 
-          // potential dynamic template string
-          if (rawUrl[0] === '`' && /\$\{/.test(rawUrl)) {
-            const ast = this.parse(rawUrl)
-            const templateLiteral = (ast as any).body[0].expression
-            if (templateLiteral.expressions.length) {
-              const pattern = buildGlobPattern(templateLiteral)
-              // Note: native import.meta.url is not supported in the baseline
-              // target so we use the global location here. It can be
-              // window.location or self.location in case it is used in a Web Worker.
-              // @see https://developer.mozilla.org/en-US/docs/Web/API/Window/self
-              s.overwrite(
-                index,
-                index + exp.length,
-                `new URL(import.meta.globEagerDefault(${JSON.stringify(
-                  pattern
-                )})[${rawUrl}], self.location)`,
-                { contentOnly: true }
-              )
-              continue
-            }
+        // potential dynamic template string
+        if (rawUrl[0] === '`' && /\$\{/.test(rawUrl)) {
+          const ast = this.parse(rawUrl)
+          const templateLiteral = (ast as any).body[0].expression
+          if (templateLiteral.expressions.length) {
+            const pattern = buildGlobPattern(templateLiteral)
+            // Note: native import.meta.url is not supported in the baseline
+            // target so we use the global location here. It can be
+            // window.location or self.location in case it is used in a Web Worker.
+            // @see https://developer.mozilla.org/en-US/docs/Web/API/Window/self
+            s.overwrite(
+              index,
+              index + exp.length,
+              `new URL(import.meta.globEagerDefault(${JSON.stringify(
+                pattern
+              )})[${rawUrl}], self.location)`,
+              { contentOnly: true }
+            )
+            continue
           }
+        }
 
-          const url = rawUrl.slice(1, -1)
-          const file = path.resolve(path.dirname(id), url)
-          // 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(() => {
-            config.logger.warnOnce(
-              `\n${exp} doesn't exist at build time, it will remain unchanged to be resolved at runtime`
-            )
-            return url
-          })
-          s.overwrite(
-            index,
-            index + exp.length,
-            `new URL(${JSON.stringify(builtUrl)}, self.location)`,
-            { contentOnly: true }
+        const url = rawUrl.slice(1, -1)
+        const file = path.resolve(path.dirname(id), url)
+        // 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 truthExp = cleanString.raw.slice(index, index + exp.length)
+          config.logger.warnOnce(
+            `\n${truthExp} doesn't exist at build time, it will remain unchanged to be resolved at runtime`
           )
-        }
-        if (s) {
-          return {
-            code: s.toString(),
-            map: config.build.sourcemap ? s.generateMap({ hires: true }) : null
-          }
+          return url
+        })
+        s.overwrite(
+          index,
+          index + exp.length,
+          `new URL(${JSON.stringify(builtUrl)}, self.location)`,
+          { contentOnly: true }
+        )
+      }
+      if (s) {
+        return {
+          code: s.toString(),
+          map: config.build.sourcemap ? s.generateMap({ hires: true }) : null
         }
       }
       return null