Custom commands shifts arguments when any value is `undefined`

[AriPerkkio]

Describe the bug

import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    browser: {
      commands: {
        readConfig(encoding: undefined | string, filepath: string) {
          // ...
        }
      },
    }
  },
});

import { commands } from "vitest/browser";

// On server side: { encoding: "utf8", filepath: "file.ts" } ✅
commands.readConfig("utf8", "file.ts");

// On server side: { encoding: "file.ts", filepath: undefined } ❌
commands.readConfig(undefined, "file.ts");

// Work-around for now:
const encodig: string | undefined = ...
commands.readConfig(encoding ?? null, "file.ts");

Reproduction

#10865

System Info

Vitest CI, Playwright provider, Vitest 4.1 + 5

Used Package Manager

pnpm

Validations

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

[nguyennvk]

Hi @AriPerkkio
I wanna propose the approach to the fix and need your idea on my approach. The issue is in file packages/browser/src/client/tester/tester-utils.ts on line 125 (triggerCommand function).

I found that args is fine at the beginning of the function but at the end, it siliently ignores undefined which is the issue that you described. The fix is to remove line 136 args = args.filter(arg => arg !== undefined) which filter out undefined value.

My question is how do you define optional fields to remove and when should i not remove undefined value.

By the way, for the test that you wrote for this issue the value of args1 should be null when instead of undefined according to my run.

[AriPerkkio]

The root cause analysis around args = args.filter(arg => arg !== undefined) sound correct. But I'm not sure what's the correct fix. I'd start by removing that line and checking what breaks - hopefully some tests start failing after that.

the test that you wrote for this issue the value of args1 should be null when instead of undefined according to my run.

I would expect the value to be undefined on server side when undefined is passed, not null. Though I'm not sure if we have any mechanisms around serializing these two differently over the websocket rpc. 🤔

[nguyennvk]

What do you think about ignoring all the trailing undefined? Like args = [undefined, "a", undefined] becomes [undefined, "a"]

What tests should I look for after removing args = args.filter(arg => arg !== undefined ?

[nguyennvk]

@AriPerkkio I updated 2 things which are deleting args = args.filter(arg => arg !== undefined) and add payload = payload.map(value => value === null ? undefined : value); to packages/browser/src/node/rpc.ts

Then i ran test:ci and it passed. Do you have any concerns with my approach?

Note: test:ci isn't idempotent locally — merge-reports and browser-persistent-context fail on a second run due to leftover blob reports and a persisted browser profile. Not related to this change.

[SnowingFox]

Opened a PR to address this: #10916