fix(browser): correctly update inline snapshot if changed (#5925)

.gitignore

@@ -23,4 +23,5 @@ docs/public/sponsors
 docs/.vitepress/cache/
 !test/cli/fixtures/dotted-files/**/.cache
 test/browser/test/__screenshots__/**/*
+test/browser/fixtures/update-snapshot/basic.test.ts
 .vitest-reports

packages/browser/src/client/tester/runner.ts

@@ -1,6 +1,9 @@
-import type { File, Task, TaskResultPack, VitestRunner } from '@vitest/runner'
+import type { File, Suite, Task, TaskResultPack, VitestRunner } from '@vitest/runner'
 import type { ResolvedConfig, WorkerGlobalState } from 'vitest'
 import type { VitestExecutor } from 'vitest/execute'
+import { NodeBenchmarkRunner, VitestTestRunner } from 'vitest/runners'
+import { loadDiffConfig, loadSnapshotSerializers, takeCoverageInsideWorker } from 'vitest/browser'
+import { TraceMap, originalPositionFor } from 'vitest/utils'
 import { importId } from '../utils'
 import { VitestBrowserSnapshotEnvironment } from './snapshot'
 import { rpc } from './rpc'
@@ -28,6 +31,7 @@ export function createBrowserRunner(
   return class BrowserTestRunner extends runnerClass implements VitestRunner {
     public config: ResolvedConfig
     hashMap = browserHashMap
+    public sourceMapCache = new Map<string, any>()
 
     constructor(options: BrowserRunnerOptions) {
       super(options.config)
@@ -48,6 +52,22 @@ export function createBrowserRunner(
       }
     }
 
+    onBeforeRunSuite = async (suite: Suite | File) => {
+      await Promise.all([
+        super.onBeforeRunSuite?.(suite),
+        (async () => {
+          if ('filepath' in suite) {
+            const map = await rpc().getBrowserFileSourceMap(suite.filepath)
+            this.sourceMapCache.set(suite.filepath, map)
+            const snapshotEnvironment = this.config.snapshotOptions.snapshotEnvironment
+            if (snapshotEnvironment instanceof VitestBrowserSnapshotEnvironment) {
+              snapshotEnvironment.addSourceMap(suite.filepath, map)
+            }
+          }
+        })(),
+      ])
+    }
+
     onAfterRunFiles = async (files: File[]) => {
       const [coverage] = await Promise.all([
         coverageModule?.takeCoverage?.(),
@@ -75,7 +95,7 @@ export function createBrowserRunner(
 
       if (this.config.includeTaskLocation) {
         try {
-          await updateFilesLocations(files)
+          await updateFilesLocations(files, this.sourceMapCache)
         }
         catch (_) {}
       }
@@ -112,13 +132,6 @@ export async function initiateRunner(
   if (cachedRunner) {
     return cachedRunner
   }
-  const [
-    { VitestTestRunner, NodeBenchmarkRunner },
-    { takeCoverageInsideWorker, loadDiffConfig, loadSnapshotSerializers },
-  ] = await Promise.all([
-    importId('vitest/runners') as Promise<typeof import('vitest/runners')>,
-    importId('vitest/browser') as Promise<typeof import('vitest/browser')>,
-  ])
   const runnerClass
     = config.mode === 'test' ? VitestTestRunner : NodeBenchmarkRunner
   const BrowserRunner = createBrowserRunner(runnerClass, mocker, state, {
@@ -141,14 +154,9 @@ export async function initiateRunner(
   return runner
 }
 
-async function updateFilesLocations(files: File[]) {
-  const { loadSourceMapUtils } = (await importId(
-    'vitest/utils',
-  )) as typeof import('vitest/utils')
-  const { TraceMap, originalPositionFor } = await loadSourceMapUtils()
-
+async function updateFilesLocations(files: File[], sourceMaps: Map<string, any>) {
   const promises = files.map(async (file) => {
-    const result = await rpc().getBrowserFileSourceMap(file.filepath)
+    const result = sourceMaps.get(file.filepath) || await rpc().getBrowserFileSourceMap(file.filepath)
     if (!result) {
       return null
     }

packages/browser/src/client/tester/snapshot.ts

@@ -1,7 +1,15 @@
 import type { SnapshotEnvironment } from 'vitest/snapshot'
+import { type ParsedStack, TraceMap, originalPositionFor } from 'vitest/utils'
 import type { VitestBrowserClient } from '../client'
 
 export class VitestBrowserSnapshotEnvironment implements SnapshotEnvironment {
+  private sourceMaps = new Map<string, any>()
+  private traceMaps = new Map<string, TraceMap>()
+
+  public addSourceMap(filepath: string, map: any) {
+    this.sourceMaps.set(filepath, map)
+  }
+
   getVersion(): string {
     return '1'
   }
@@ -29,6 +37,23 @@ export class VitestBrowserSnapshotEnvironment implements SnapshotEnvironment {
   removeSnapshotFile(filepath: string): Promise<void> {
     return rpc().removeSnapshotFile(filepath)
   }
+
+  processStackTrace(stack: ParsedStack): ParsedStack {
+    const map = this.sourceMaps.get(stack.file)
+    if (!map) {
+      return stack
+    }
+    let traceMap = this.traceMaps.get(stack.file)
+    if (!traceMap) {
+      traceMap = new TraceMap(map)
+      this.traceMaps.set(stack.file, traceMap)
+    }
+    const { line, column } = originalPositionFor(traceMap, stack)
+    if (line != null && column != null) {
+      return { ...stack, line, column }
+    }
+    return stack
+  }
 }
 
 function rpc(): VitestBrowserClient['rpc'] {

packages/browser/src/client/vite.config.ts

@@ -27,7 +27,7 @@ export default defineConfig({
       name: 'virtual:msw',
       enforce: 'pre',
       resolveId(id) {
-        if (id.startsWith('msw') || id.startsWith('vitest')) {
+        if (id.startsWith('msw') || id.startsWith('vitest') || id.startsWith('@vitest/browser')) {
           return `/__virtual_vitest__?id=${encodeURIComponent(id)}`
         }
       },

packages/browser/src/node/index.ts

@@ -45,9 +45,6 @@ export async function createBrowserServer(
   await vite.listen()
 
   setupBrowserRpc(server)
-  // if (project.config.browser.ui) {
-  //   setupUiRpc(project.ctx, server)
-  // }
 
   return server
 }

packages/browser/src/node/plugin.ts

@@ -127,6 +127,7 @@ export default (browserServer: BrowserServer, base = '/'): Plugin[] => {
               'vitest/browser',
               'vitest/runners',
               '@vitest/utils',
+              '@vitest/utils/source-map',
               '@vitest/runner',
               '@vitest/spy',
               '@vitest/utils/error',

packages/browser/src/node/pool.ts

@@ -42,7 +42,7 @@ export function createBrowserPool(ctx: Vitest): ProcessPool {
 
     if (!origin) {
       throw new Error(
-        `Can't find browser origin URL for project "${project.config.name}"`,
+        `Can't find browser origin URL for project "${project.getName()}" when running tests for files "${files.join('", "')}"`,
       )
     }
 

packages/snapshot/src/port/inlineSnapshot.ts

@@ -23,7 +23,7 @@ export async function saveInlineSnapshots(
   await Promise.all(
     Array.from(files).map(async (file) => {
       const snaps = snapshots.filter(i => i.file === file)
-      const code = (await environment.readSnapshotFile(file)) as string
+      const code = await environment.readSnapshotFile(file) as string
       const s = new MagicString(code)
 
       for (const snap of snaps) {
@@ -116,22 +116,46 @@ function prepareSnapString(snap: string, source: string, index: number) {
     .replace(/\$\{/g, '\\${')}\n${indent}${quote}`
 }
 
+const toMatchInlineName = 'toMatchInlineSnapshot'
+const toThrowErrorMatchingInlineName = 'toThrowErrorMatchingInlineSnapshot'
+
+// on webkit, the line number is at the end of the method, not at the start
+function getCodeStartingAtIndex(code: string, index: number) {
+  const indexInline = index - toMatchInlineName.length
+  if (code.slice(indexInline, index) === toMatchInlineName) {
+    return {
+      code: code.slice(indexInline),
+      index: indexInline,
+    }
+  }
+  const indexThrowInline = index - toThrowErrorMatchingInlineName.length
+  if (code.slice(index - indexThrowInline, index) === toThrowErrorMatchingInlineName) {
+    return {
+      code: code.slice(index - indexThrowInline),
+      index: index - indexThrowInline,
+    }
+  }
+  return {
+    code: code.slice(index),
+    index,
+  }
+}
+
 const startRegex
   = /(?:toMatchInlineSnapshot|toThrowErrorMatchingInlineSnapshot)\s*\(\s*(?:\/\*[\s\S]*\*\/\s*|\/\/.*(?:[\n\r\u2028\u2029]\s*|[\t\v\f \xA0\u1680\u2000-\u200A\u202F\u205F\u3000\uFEFF]))*[\w$]*(['"`)])/
 export function replaceInlineSnap(
   code: string,
   s: MagicString,
-  index: number,
+  currentIndex: number,
   newSnap: string,
 ) {
-  const codeStartingAtIndex = code.slice(index)
+  const { code: codeStartingAtIndex, index } = getCodeStartingAtIndex(code, currentIndex)
 
   const startMatch = startRegex.exec(codeStartingAtIndex)
 
-  const firstKeywordMatch
-    = /toMatchInlineSnapshot|toThrowErrorMatchingInlineSnapshot/.exec(
-      codeStartingAtIndex,
-    )
+  const firstKeywordMatch = /toMatchInlineSnapshot|toThrowErrorMatchingInlineSnapshot/.exec(
+    codeStartingAtIndex,
+  )
 
   if (!startMatch || startMatch.index !== firstKeywordMatch?.index) {
     return replaceObjectSnap(code, s, index, newSnap)

packages/snapshot/src/port/state.ts

@@ -140,18 +140,20 @@ export default class SnapshotState {
   ): void {
     this._dirty = true
     if (options.isInline) {
+      const error = options.error || new Error('snapshot')
       const stacks = parseErrorStacktrace(
-        options.error || new Error('snapshot'),
+        error,
         { ignoreStackEntries: [] },
       )
-      const stack = this._inferInlineSnapshotStack(stacks)
-      if (!stack) {
+      const _stack = this._inferInlineSnapshotStack(stacks)
+      if (!_stack) {
         throw new Error(
           `@vitest/snapshot: Couldn't infer stack frame for inline snapshot.\n${JSON.stringify(
             stacks,
           )}`,
         )
       }
+      const stack = this.environment.processStackTrace?.(_stack) || _stack
       // removing 1 column, because source map points to the wrong
       // location for js files, but `column-1` points to the same in both js/ts
       // https://github.com/vitejs/vite/issues/8657

packages/snapshot/src/types/environment.ts

@@ -1,3 +1,5 @@
+import type { ParsedStack } from '@vitest/utils'
+
 export interface SnapshotEnvironment {
   getVersion: () => string
   getHeader: () => string
@@ -6,6 +8,7 @@ export interface SnapshotEnvironment {
   saveSnapshotFile: (filepath: string, snapshot: string) => Promise<void>
   readSnapshotFile: (filepath: string) => Promise<string | null>
   removeSnapshotFile: (filepath: string) => Promise<void>
+  processStackTrace?: (stack: ParsedStack) => ParsedStack
 }
 
 export interface SnapshotEnvironmentOptions {

packages/vitest/src/public/utils.ts

@@ -1,5 +1,2 @@
 export * from '@vitest/utils'
-
-export function loadSourceMapUtils() {
-  return import('@vitest/utils/source-map')
-}
+export * from '@vitest/utils/source-map'

test/browser/fixtures/update-snapshot/__snapshots__/custom/my_snapshot

@@ -0,0 +1 @@
+my snapshot content

test/browser/fixtures/update-snapshot/basic-fixture.ts

@@ -0,0 +1,27 @@
+import { expect, test, vi } from 'vitest'
+
+interface _BasicInterface {
+  willBeRemoved: boolean
+  leavingSourceMapIncorrect: boolean
+}
+
+test('inline snapshot', () => {
+  expect(1).toMatchInlineSnapshot('1')
+})
+
+test('basic', () => {
+  expect(1).toMatchSnapshot()
+})
+
+test('renders inline mock snapshot', () => {
+  const fn = vi.fn()
+  expect(fn).toMatchInlineSnapshot()
+  fn('hello', 'world', 2)
+  expect(fn).toMatchInlineSnapshot()
+})
+
+test('file snapshot', async () => {
+  await expect('my snapshot content')
+    .toMatchFileSnapshot('./__snapshots__/custom/my_snapshot')
+})
+

test/browser/fixtures/update-snapshot/vitest.config.ts

@@ -7,7 +7,7 @@ manually test snapshot by
   pnpm -C test/browser test-fixtures --root fixtures/update-snapshot
 */
 
-const provider = process.env.PROVIDER || 'webdriverio'
+const provider = process.env.PROVIDER || 'playwright'
 const browser =
   process.env.BROWSER || (provider === 'playwright' ? 'chromium' : 'chrome')
 

test/browser/package.json

@@ -10,6 +10,7 @@
     "test:safaridriver": "PROVIDER=webdriverio BROWSER=safari pnpm run test:unit",
     "test-fixtures": "vitest",
     "test-mocking": "vitest --root ./fixtures/mocking",
+    "test-snapshots": "vitest --root ./fixtures/update-snapshot",
     "coverage": "vitest --coverage.enabled --coverage.provider=istanbul --browser.headless=yes",
     "test:browser:playwright": "PROVIDER=playwright vitest",
     "test:browser:webdriverio": "PROVIDER=webdriverio vitest"

test/browser/specs/__snapshots__/update-snapshot.test.ts.snap

@@ -0,0 +1,48 @@
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+exports[`update snapshot 1`] = `
+"import { expect, test, vi } from 'vitest'
+
+interface _BasicInterface {
+  willBeRemoved: boolean
+  leavingSourceMapIncorrect: boolean
+}
+
+test('inline snapshot', () => {
+  expect(1).toMatchInlineSnapshot('1')
+})
+
+test('basic', () => {
+  expect(1).toMatchSnapshot()
+})
+
+test('renders inline mock snapshot', () => {
+  const fn = vi.fn()
+  expect(fn).toMatchInlineSnapshot(\`[MockFunction spy]\`)
+  fn('hello', 'world', 2)
+  expect(fn).toMatchInlineSnapshot(\`
+    [MockFunction spy] {
+      "calls": [
+        [
+          "hello",
+          "world",
+          2,
+        ],
+      ],
+      "results": [
+        {
+          "type": "return",
+          "value": undefined,
+        },
+      ],
+    }
+  \`)
+})
+
+test('file snapshot', async () => {
+  await expect('my snapshot content')
+    .toMatchFileSnapshot('./__snapshots__/custom/my_snapshot')
+})
+
+"
+`;