Skip to content

Conversation

@sirlancelot
Copy link
Contributor

@sirlancelot sirlancelot commented Nov 7, 2025

I added two new features for increased feature parity with testdouble.js:

  1. ignoreExtraArgs: true - Behavior matches testdouble.js exactly: https://github.com/testdouble/testdouble.js/blob/main/docs/5-stubbing-results.md#ignoreextraargs
  2. .thenCallback() - Behavior mostly matches testdouble.js with two insignificant differences: https://github.com/testdouble/testdouble.js/blob/main/docs/5-stubbing-results.md#stubbing-callback-apis
    1. When explicitly adding the callback matcher, you must always call it even with zero arguments.
    2. The callbacks are always invoked asynchronously. This eliminates the need for adding defer: true and enforces correct async behavior.

The new features are fully tested and documented.

As an added bonus, I adjusted the debug() function to put a little checkmark next to stubs that were successfully invoked. testdouble.js doesn't have this but I thought it would help with debugging. Rather than two lists of matched and unmatched calls, it's now one list in the precise invocation order.

@sirlancelot sirlancelot force-pushed the feature/testdouble-features branch from cad51f8 to f4a06fc Compare November 7, 2025 20:10
@mcous
Copy link
Owner

mcous commented Nov 7, 2025

Hello again @sirlancelot! Could you do me a favor and split these up into one change per PR?

  1. debug improvements
  2. ignoreExtraArgs
  3. thenCallback

I'm always here for debug improvements, and ignoreExtraArgs is fun so I want to give this feature its own attention so we can get the types correct. As an initial gut reaction, though, I don't think thenCallback is a meaningful enough improvement over thenDo to be worth the added complexity. Plus:

  • I don't want to get into the confusion of "is this async or not?" given that a synchronous callback is a perfectly valid API design decision
  • In my experience, import 'vitest' does not work well inside of libraries, and I don't want to be implicitly extending expect to add a expect.callback matcher

Callbacks can already be accomplished with:

// synchronous
const mock = when(vi.fn())
  .calledWith('data', expect.any(Function))
  .thenDo((_, callback) => callback('result'))

const cb = vi.fn()
mock('data', cb)

expect(cb).toHaveBeenCalledWith('result')
// asynchronous
const mock = when(vi.fn())
  .calledWith('data', expect.any(Function))
  .thenDo((_, callback) => setImmediate(() => callback('result')));

const cb = vi.fn()
mock('data', cb)

await waitFor(() => {
  expect(cb).toHaveBeenCalledWith('result')
})

If possible, can you also try to keep unrelated refactoring to a minimum? We've all got our own coding styles, and I'd like to stick with mine in this project if at all possible 😄. For future reference, feel free to open an issue first for any features you want to propose! I'd rather chat before you write any code so it doesn't feel like I'm shooting your work down

@sirlancelot
Copy link
Contributor Author

I totally understand. This PR kind of snowballed while I was tinkering on slow work day. I figured it would be too big to be wholesale accepted, but figured I'd show my work anyway. 😅

For what it's worth, I attempted to follow the style as closely as possible which, thanks to ESLint & Prettier, it was fairly straightforward. The refactorings were intentional to both improve performance (save iterating through a list twice), and readability with fewer object transformations.

I'll split these features in to separate PRs though so it's easier to review.

@sirlancelot sirlancelot closed this Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants