Read window.rAF later to allow fake timers to work correctly
#4701
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
I'm just trying to figure out how I can write a test for this |
ensconced
changed the title
✅ Deploy Preview for redux-starter-kit-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3907062:
|
| typeof window !== 'undefined' && window.requestAnimationFrame | ||
| ? window.requestAnimationFrame | ||
| : createQueueWithTimer(10) | ||
|
|
There was a problem hiding this comment.
At this point, this could probably be inlined in line 69.
There was a problem hiding this comment.
Makes sense, I have pushed this change.
ensconced
force-pushed
the
master
branch
2 times, most recently
from
854a592 to
fe61d49
Compare
…h fake timers)
| @@ -125,3 +125,84 @@ describe.each(cases)('autoBatchEnhancer: %j', (autoBatchOptions) => { | |||
| expect(subscriptionNotifications).toBe(3) | |||
| }) | |||
| }) | |||
|
|
|||
| describe.each(cases)( | |||
There was a problem hiding this comment.
Two of these tests were failing before the fix was applied
| @@ -66,7 +59,8 @@ export const autoBatchEnhancer = | |||
| options.type === 'tick' | |||
| ? queueMicrotask | |||
| : options.type === 'raf' | |||
| ? rAF | |||
| ? // requestAnimationFrame won't exist in SSR environments. Fall back to a vague approximation just to keep from erroring. | |||
| window?.requestAnimationFrame ?? createQueueWithTimer(10) | |||
There was a problem hiding this comment.
window? doesn't cover all cases that typeof window !== 'undefined' does - optional chaining still can cause a reference error if a variable with that name never was defined:
There was a problem hiding this comment.
ah, of course. I have just pushed a fix to revert it back to a ternary like before
reading window.requestAnimationFrame within the autoBatchEnhancer function instead of in the top-level module scope. This ensures that we use the fake version of requestAnimationFrame if jest is using fake timers.
|
Let me know if there's anything else I can do to help this get merged 🙂 |
|
Mostly, "make the day have 48 hours" :) Frankly, we have a ton to do (here and in our day-to-day lifes), so PRs can sit around for quite a while until one of us finds the time to thoroughly go over a few of them. At first glance, this looks good, but please don't expect this to be merged/released soon. You can use the CodeSandbox release from the CSB comment in the meantime! |
markerikson
changed the title
window.rAF later to allow fake timers to work correctly
This fixes issue #4693 by reading window.requestAnimationFrame within the autoBatchEnhancer function instead of in the top-level module scope. This ensures that we use the fake version of requestAnimationFrame if jest is using fake timers.