sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
793 stars 103 forks source link

fix: requestAnimationFrame args #458

Closed CreativeTechGuy closed 1 year ago

CreativeTechGuy commented 1 year ago

Purpose (TL;DR)

Fix issue #454 by computing requestAnimationFrame args at the time when the callback is fired.

codecov[bot] commented 1 year ago

Codecov Report

Base: 95.52% // Head: 95.53% // Increases project coverage by +0.00% :tada:

Coverage data is based on head (cf18928) compared to base (ad1d43c). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #458 +/- ## ======================================= Coverage 95.52% 95.53% ======================================= Files 2 2 Lines 648 649 +1 ======================================= + Hits 619 620 +1 Misses 29 29 ``` | Flag | Coverage Δ | | |---|---|---| | unit | `95.53% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/sinonjs/fake-timers/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | | |---|---|---| | [src/fake-timers-src.js](https://codecov.io/gh/sinonjs/fake-timers/pull/458/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-c3JjL2Zha2UtdGltZXJzLXNyYy5qcw==) | `95.52% <100.00%> (+<0.01%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

CreativeTechGuy commented 1 year ago

For the record, I am preserving the existing behavior which passes clock.now in the case where performance.now() isn't available. But that should probably be changed too since that doesn't match the browser's behavior. Should we break out the function that is used to implement performance.now() to be usable even when that method isn't mocked? That way requestAnimationFrame's args could be accurate in all cases?

fatso83 commented 1 year ago

Should we break out the function that is used to implement performance.now() to be usable even when that method isn't mocked?

Yes, I think that makes perfect sense! Separate commit and separate test case :)

CreativeTechGuy commented 1 year ago

You got it @fatso83! New commit and updated tests. Hope you like it 😃

CreativeTechGuy commented 1 year ago

@fatso83 Let me know if the latest changes address all of your concerns. I think this is good to go!

SimenB commented 1 year ago

Lockfile was changed from v2 to v1, probably not on purpose 🙂

CreativeTechGuy commented 1 year ago

@SimenB Oh no! I'm not even sure how that could have accidentally happened. I would have understood if it upgraded to v3 somehow, but downgraded? No clue! I blame GitHub Codespaces' auto-setup haha.

How would you like me to fix it? I definitely didn't intend to change anything in that file, I'm so sorry it got committed.

SimenB commented 1 year ago

I wouldn't worry about it, it doesn't really matter. Just a bit of extra diff 👍

CreativeTechGuy commented 1 year ago

I tried to reproduce it with a fresh GitHub Codespace and instantly upon setup it converted it back to v2 so I have no clue what I did. I guess whoever does the next PR will likely (either intentionally or accidentally) upgrade it to v2 again! haha