search

jeremenichelli / hunt

👻 Minimal library to observe nodes entering and leaving the viewport
https://jeremenichelli.github.io/hunt
MIT License
342 stars 10 forks source link

Ensure throttle always invokes the function at the end of the throttle interval #20

Closed markgoodyear closed 7 years ago

markgoodyear commented 7 years ago

When scrolling too fast, the current throttle could miss invoking the function if its still within the interval. I'm using hunt for lazy loading content, and every so often hunt will miss the observable element when scrolling too fast. This PR will ensure that the the function is called after the throttle interval, so even when scrolling too fast, it'll still be called.

Another option is to use requestAnimationFrame and let the browser handle when to update.

EDIT: Credit to the function in this post: https://medium.com/@_jh3y/throttling-and-debouncing-in-javascript-b01cad5c8edf — I just modified it to fit in the eslint rules.

jeremenichelli commented 7 years ago

Hi Mark, thanks a lot for the PR.

Have you checked the test.html with this change? I want to make sure performance is not critically hit by the throttling change, since the initial idea was to bypass actions if the scrolling was too fast, though I understand it could bring inconsistency for some cases like yours.

About the performance, I've done tests with requestAnimationFrame and because it queues the function as a high priority action it misses the 60fps while scrolling, but I'm open to suggestion or other approaches like Daniel did here: https://github.com/jeremenichelli/hunt/pull/19

markgoodyear commented 7 years ago

Hey @jeremenichelli,

I had a look and it looked to run fine — the FPS was the same as the current throttle for me, both between 54-60FPS. I am running a high spec Mac hooked up to a retina 27" monitor, so it may need checking elsewhere too. I believe the throttle is implemented in a similar way in Lodash/Underscore.

jeremenichelli commented 7 years ago

Nice, will take a look at it locally and see if I don't spot anything odd and will merge.

markgoodyear commented 7 years ago

@jeremenichelli Awesome. I tested rAF too, but the frame rate drops lower than both throttle methods, dipped below 50FPS. Not tested on mobile though so I'll have a test when I get chance.

jeremenichelli commented 7 years ago

I'm seeing a little bit downgrade in performance but on heavy limit cases, since to be working fine. I will merge and publish tomorrow morning. Thanks a lot for the fix Mark!