fix(browser): show a helpful error when spying on an export (#8178)

Author: sheremet-va

docs/guide/browser/index.md

@@ -584,3 +584,45 @@ test('renders a message', async () => {
 When using Vitest Browser, it's important to note that thread blocking dialogs like `alert` or `confirm` cannot be used natively. This is because they block the web page, which means Vitest cannot continue communicating with the page, causing the execution to hang.
 
 In such situations, Vitest provides default mocks with default returned values for these APIs. This ensures that if the user accidentally uses synchronous popup web APIs, the execution would not hang. However, it's still recommended for the user to mock these web APIs for better experience. Read more in [Mocking](/guide/mocking).
+
+### Spying on Module Exports
+
+Browser Mode uses the browser's native ESM support to serve modules. The module namespace object is sealed and can't be reconfigured, unlike in Node.js tests where Vitest can patch the Module Runner. This means you can't call `vi.spyOn` on an imported object:
+
+```ts
+import { vi } from 'vitest'
+import * as module from './module.js'
+
+vi.spyOn(module, 'method') // ❌ throws an error
+```
+
+To bypass this limitation, Vitest supports `{ spy: true }` option in `vi.mock('./module.js')`. This will automatically spy on every export in the module without replacing them with fake ones.
+
+```ts
+import { vi } from 'vitest'
+import * as module from './module.js'
+
+vi.mock('./module.js', { spy: true })
+
+vi.mocked(module.method).mockImplementation(() => {
+  // ...
+})
+```
+
+However, the only way to mock exported _variables_ is to export a method that will change the internal value:
+
+::: code-group
+```js [module.js]
+export let MODE = 'test'
+export function changeMode(newMode) {
+  MODE = newMode
+}
+```
+```js [module.test.ts]
+import { expect } from 'vitest'
+import { changeMode, MODE } from './module.js'
+
+changeMode('production')
+expect(MODE).toBe('production')
+```
+:::

eslint.config.js

@@ -113,6 +113,7 @@ export default antfu(
       'unused-imports/no-unused-imports': 'off',
       'ts/method-signature-style': 'off',
       'no-self-compare': 'off',
+      'import/no-mutable-exports': 'off',
     },
   },
   {

packages/spy/src/index.ts

@@ -467,14 +467,34 @@ export function spyOn<T, K extends keyof T>(
     state = fn.mock._state()
   }
 
-  const stub = tinyspy.internalSpyOn(obj, objMethod as any)
-  const spy = enhanceSpy(stub) as MockInstance
+  try {
+    const stub = tinyspy.internalSpyOn(obj, objMethod as any)
 
-  if (state) {
-    spy.mock._state(state)
+    const spy = enhanceSpy(stub) as MockInstance
+
+    if (state) {
+      spy.mock._state(state)
+    }
+
+    return spy
   }
+  catch (error) {
+    if (
+      error instanceof TypeError
+      && Symbol.toStringTag
+      && (obj as any)[Symbol.toStringTag] === 'Module'
+      && (error.message.includes('Cannot redefine property')
+        || error.message.includes('Cannot replace module namespace')
+        || error.message.includes('can\'t redefine non-configurable property'))
+    ) {
+      throw new TypeError(
+        `Cannot spy on export "${String(objMethod)}". Module namespace is not configurable in ESM. See: https://vitest.dev/guide/browser/#limitations`,
+        { cause: error },
+      )
+    }
 
-  return spy
+    throw error
+  }
 }
 
 let callOrder = 0

test/browser/specs/runner.test.ts

@@ -1,5 +1,6 @@
 import type { Vitest } from 'vitest/node'
 import type { JsonTestResults } from 'vitest/reporters'
+import { readdirSync } from 'node:fs'
 import { readFile } from 'node:fs/promises'
 import { beforeAll, describe, expect, onTestFailed, test } from 'vitest'
 import { rolldownVersion } from 'vitest/node'
@@ -68,10 +69,11 @@ describe('running browser tests', async () => {
     expect(vitest.projects.map(p => p.browser?.vite.config.optimizeDeps.entries))
       .toEqual(vitest.projects.map(() => expect.arrayContaining(testFiles)))
 
-    // This should match the number of actual tests from browser.json
-    // if you added new tests, these assertion will fail and you should
-    // update the numbers
-    expect(browserResultJson.testResults).toHaveLength(16 * instances.length)
+    const testFilesCount = readdirSync('./test')
+      .filter(n => n.includes('.test.'))
+      .length + 1 // 1 is in-source-test
+
+    expect(browserResultJson.testResults).toHaveLength(testFilesCount * instances.length)
     expect(passedTests).toHaveLength(browserResultJson.testResults.length)
     expect(failedTests).toHaveLength(0)
   })

test/browser/test/mocking.test.ts

@@ -0,0 +1,18 @@
+import { expect, it, vi } from 'vitest'
+import * as module from '../src/calculator'
+
+it('spying on an esm module prints an error', () => {
+  const error: Error = (() => {
+    try {
+      vi.spyOn(module, 'calculator')
+      expect.unreachable()
+    }
+    catch (err) {
+      return err
+    }
+  })()
+  expect(error.name).toBe('TypeError')
+  expect(error.message).toMatchInlineSnapshot(`"Cannot spy on export "calculator". Module namespace is not configurable in ESM. See: https://vitest.dev/guide/browser/#limitations"`)
+
+  expect(error.cause).toBeInstanceOf(TypeError)
+})