Open AlexandrHoroshih opened 1 month ago
We are using @sinon/fake-timers
for timers mocking, so it would have to be implemented by them because we don't override global timers access. Or we would have to reimplement mocking ourselves which is also an option.
In any case, we can't do anything until the AsyncContext
API is implemented at least in one of the browsers and Node.js.
because we don't override global timers access
Sorry, but vitest definitely overrides global functions. Using @sinon/fake-timers
's withGlobal
, but with globalThis
nonetheless.
because we don't override global timers access
Sorry, but vitest definitely overrides global functions. Using
@sinon/fake-timers
'swithGlobal
, but withglobalThis
nonetheless.
What I mean is that we don't override it (Vitest in its source code doesn't reassign globalThis.setTimeout
to control how it works, so we can't inject the context there), not that we don't override it.
Clear and concise description of the problem
Hello and thanks for the great project! 💙
Small disclaimer
I have seen comments in the #5665 and i'm aware that Vitest
But i think that this API idea might be useful, once
AsyncContext
proposal will be implemented in the browsers sometime in the future, so i decided to propose it anyway 😅The problem
Current time mocks API has a noticeable disadvantage: The
vi.useFakeTimers()/useRealTimers()
switch is global, at least for all tests in the current worker thread/child processThis, for example, may lead to problems, especially when running tests concurrently - since fake timers are not isolated between tests, they may leak between those and cause unexpected and confusing behaviour
As a side note: i probably would prefer, if
vitest
had banned using globaluseFakeTimers()
switch along withtest.concurrent
in the same test suite 🤔And even if we rule out
.concurrent
tests as a special case, the global switch behaviour is still may be quite confusing at times, since there are cases likesetTimeout
before any mocking code was run, so user ends up with two concurrent time flows in the same test 🤯useRealTimers
switch was not called at the end of the test, next one also gets fake time anyway, which also may lead to unexpected and confusing behaviour, depending on theuseFakeTimers
configSuggested solution
So, since the core of the problem is the "global switch" nature of current fake timers API, the alternative must be "local" for the each test
I suggest API like this:
☝️ In this example it is expected, that time would be mocked only inside
runWithFakeTime
callback and any of event-loop tasks which were produced by it, which obviously requires API likeAsyncContext
To do that all of the time-related APIs must be mocked in a way, that they use fake implementation if called inside
runWithFakeTime
context and native one if called outside of it And i think, this mock should happpen before any other userland code in the suite was initialized, so any code, that saves separate reference to e.g.setTimeout
, still gets mocked as expectedUnlike global switch, this API would allow for clear isolation of time mocks from any other code in the same suite, since "fake time zone" is separated into its own async context
Here is a little Proof-of-Concept example, which uses Node.js
AsyncLocalStorage
for this purpose: https://stackblitz.com/edit/vitejs-vite-4hqcwk?file=tests%2Flib.jsAlternative
No response
Additional context
I have seen comments in the #5665 and i'm aware that Vitest
But i think that this API idea might be useful, once
AsyncContext
proposal will be implemented in the browsers sometime in the futureSo i guess, this idea can wait until then
Validations