Cannot read properties of undefined (reading 'navigator') when using `@vitest/browser`

nicojs commented 1 month ago

Describe the bug

When running vitest inside a mocha test suite, the error "Cannot read properties of undefined (reading 'navigator')" is thrown from a global afterEach handler registered by @vitest/browser.

For example:

   ✔ should be able to start and close (1442ms)
    1) "after each" hook for "should be able to start and close"

  1 passing (1s)
  1 failing

  1) "after each" hook for "should be able to start and close":
     TypeError: Cannot read properties of undefined (reading 'navigator')
      at resetClipboardStubOnView (file:///home/nicojs/github/tmp/node_modules/@vitest/browser/dist/index.js:1058:32)
      at Context.<anonymous> (file:///home/nicojs/github/tmp/node_modules/@vitest/browser/dist/index.js:1069:21)
      at process.processImmediate (node:internal/timers:478:21)
      at process.callbackTrampoline (node:internal/async_hooks:130:17)

I've investigated this somewhat, and vitest includes some reset logic for mocking in its index.js bundle. However, this should not be included in the server-side code as it assumes running in the browser (as it tries to read from globalThis.window.navigator).

I cannot find this code inside the vitest monorepo, so I think it is a dependency that gets bundled in via rollup.

Code that causes the issue

```js const ClipboardStubControl = Symbol('Manage ClipboardSub'); function isClipboardStub(clipboard) { return !!(clipboard === null || clipboard === void 0 ? void 0 : clipboard[ClipboardStubControl]); } function resetClipboardStubOnView(window) { if (isClipboardStub(window.navigator.clipboard)) { window.navigator.clipboard[ClipboardStubControl].resetClipboardStub(); } } function detachClipboardStubFromView(window) { if (isClipboardStub(window.navigator.clipboard)) { window.navigator.clipboard[ClipboardStubControl].detachClipboardStub(); } } const g = globalThis; /* istanbul ignore else */ if (typeof g.afterEach === 'function') { g.afterEach(()=>resetClipboardStubOnView(globalThis.window)); } /* istanbul ignore else */ if (typeof g.afterAll === 'function') { g.afterAll(()=>detachClipboardStubFromView(globalThis.window)); } ```

Reproduction

I've created a simple reproduction repo for this: vitest-issue.zip

The basics is that I've created a simple mocha test that starts and stops vitest. In the vitest config I've configured browser mode.

// mocha test
import { createVitest } from 'vitest/node';
describe('test vitest', () => {

    it('should be able to start and close', async () => {
        const server = await createVitest('test', {});
        await server.start();
        await server.close();
    });
});

// vitest.config.js

import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    globals: true,
    browser: {
      enabled: true,
      name: "chromium",
      provider: "playwright",
      headless: true,
    },
    root: "vitest-test",
  },
});

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (16) x64 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
    Memory: 5.37 GB / 15.49 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 20.15.0 - ~/.nvm/versions/node/v20.15.0/bin/node
    npm: 10.7.0 - ~/.nvm/versions/node/v20.15.0/bin/npm
    pnpm: 9.4.0 - ~/.nvm/versions/node/v20.15.0/bin/pnpm
  Browsers:
    Chrome: 126.0.6478.126
  npmPackages:
    @vitest/browser: ^2.0.3 => 2.0.3 
    vitest: ^2.0.3 => 2.0.3

Used Package Manager

npm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

nicojs commented 1 month ago

I don't think importing from vitest (which also imports from @vitest/browser by extension) should have side effects at all, correct?

nicojs commented 1 month ago

Excellent, thanks, @sheremet-va, for fixing it so fast 👐 I can confirm that it works (although I have no clue why, looking at the code 😅).

sheremet-va commented 1 month ago

Excellent, thanks, @sheremet-va, for fixing it so fast 👐 I can confirm that it works (although I have no clue why, looking at the code 😅).

Rollup sees that it can't remove the file content because there is a side effect of calling a method on a globalThis. We are now saying to Rollup that it's OK to remove the file even if it detects side effects (basically, we override its detection)

vitest-dev / vitest