fix(browser): preserve undefined command arguments - #10923
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Arguments contains undefined can be missed pushing other arguments forward. close vitest-dev#10864
a2ac921 to
ae2f72f
Compare
| } | ||
| }, | ||
| async triggerCommand(sessionId, command, testPath, payload) { | ||
| payload = payload.map(value => value === null ? undefined : value) |
There was a problem hiding this comment.
This is not where the fix should be. We should fix the serialisation instead (serialise/deserialise that we pass to birpc)
There was a problem hiding this comment.
The serialization and deserialization is implemented in rpc.ts. I found that the serialization used functions from a package name "flatted". In order to achieve what we want, those functions need to be reimplemented.
There was a problem hiding this comment.
Yes, both stringify and parse accept a custom serializer. We could maybe provide a "hidden" value like "__VITEST_UNDEFINED__" to transform into undefined? Although I thought that flatted already supports undefined 🤔
3 participants
Description
Custom browser commands CANNOT read
undefinedarguments crossing the browser → Node RPC boundary. The entry was dropped from the argument array, shifting later arguments left. Example:Cause
Line 136 in packages/browser/src/client/tester/tester-utils.ts
This line filter out undefined values.
Fix
Remove that line and add a mapping in packages/browser/src/node/rpc.ts
to map null to undefined to match the test case created by @AriPerkkio
Tests
Includes the failing test from #10865 (authored by @AriPerkkio), which
now passes.
Resolves #10864
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.