-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use 'requestIdleCallback' when available for debounce/throttle #1352
Conversation
This is a severe problem in affected configurations, so I cherry-picked this commit and created a PR for 0.4 as well. To reproduce, simply scroll through an This may be dependent on the grid complexity - I have a test case I can share on request. It is likely that a Also, note that this is probably a less than optimal fix. We are basically overriding the browser's attempted performance optimization and saying "run this code anyway". It's up to the developer employing dgrid to test performance, notice issues, and adjust application configuration (including dgrid options) to alleviate them. If we fully follow the intended use of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unfortunate we have to duplicate the full blocks of code for what is really 1-2 lines of different code based on which method to follow. Is that because we effectively call the methods within the setTimeout and requestIdleCallback, or could the has statements just wrap the 1-2 lines per method that deviate between implementations?
Note, after this lands, it's time for 1.12.0, 1.11.x, 1.10.x, and 0.4.x releases!
Couldn't we abstract the decision between var delayCallback;
var cancelDelay;
if (has('requestidlecallback')) {
delayCallback = function (callback, delay) {
return requestIdleCallback(callback, { timeout: delay });
};
cancelDelay = cancelIdleCallback;
}
else {
delayCallback = setTimeout;
cancelDelay = clearTimeout;
} Then the implementations of |
I'm sold on Bryan's suggestion - performance impact of a single extra layer of function invocation is likely to be negligible. PR updated. |
Thanks @msssk! |
Fixes #1351