w3c / requestidlecallback

Cooperative Scheduling of Background Tasks
https://w3c.github.io/requestidlecallback/
Other
50 stars 19 forks source link

Collapse IdleDeadline's deadline and isExceeded into IdleDeadline.timeRemaining() #12

Closed rmcilroy closed 9 years ago

rmcilroy commented 9 years ago

Replace IdleDeadline.deadline and IdleDeadline.isExceeded() with IdleDeadline.timeRemaining()

Simplifies the API as discussed in #11

rmcilroy commented 9 years ago

Ahh right, in that case sounds good. Updated to a DOMHighResolutionTimestamp.

igrigorik commented 9 years ago

LGTM. @toddreifsteck @eliperelman any objections?

rmcilroy commented 9 years ago

I made a couple of additional changes in https://github.com/rmcilroy/requestidlecallback/commit/ca61eebda7183b6e3014d0b473a63f4c1327e1d6 based on feedback from @esprehn:

PTAL, thanks.

igrigorik commented 9 years ago

Looks good!

toddreifsteck commented 9 years ago

LGTM. @eliperelman, you ok with this?

eliperelman commented 9 years ago

+1 from me!

igrigorik commented 9 years ago

Then let it be so! Thanks guys :)

igrigorik commented 9 years ago

Why did you replace the isExceeded() method with the timeRemaining attribute? That makes the implementation impossible to polyfill.source

That's a gotcha I didn't consider. Should we go back to a method?

rmcilroy commented 9 years ago

You should be able to use a getter to implement the timeRemaining attribute in a pollyfill. For example, something like:

function IdleCallbackDeadline(deadline) { var idleDeadline = { get timeRemaining() { return deadline - performance.now(); } }; return idleDeadline; }; var deadline = IdleCallbackDeadline(performance.now() + 10000); console.log(deadline.timeRemaining);

So I don't think it is necessary to go back to a method to make it possible to polyfill.

joshduck commented 9 years ago

Using getters won't work for IE8. Globally that browser doesn't have a huge market share, but in some niches they are still important; which is a consideration when using requestIdleCallback in a framework or library.

Using a method would mirror the Date.now() or performance.now() functions. And I'd argue that a method is more appropriate for a value that is expected to change. Otherwise you end up with code that looks like it can safely be refactored like this:

if (deadline.timeRemaining > 0) doFoo(); if (deadline.timeRemaining > 0) doBar();

rmcilroy commented 9 years ago

I didn't realize IE8 doesn't support getters. However, it doesn't have requestAnimationFrame or performance.now() either, so I'm not sure how you would polyfill it on IE8 in such a way as to get any reasonable value for the timeRemaining attribute either way. What did you have in mind for an IE8 polyfill?

Maybe it would be better just to use setTimeout to polyfill and have the callbacks be able to deal with the deadline argumnent possibly being undefined, rather than trying to create an IdleCallbackDeadline object which might not accurately reflect deadlines?

joshduck commented 9 years ago

Having to handle undefined in all client code would be confusing. The callback would have to have it's own logic for continuing or terminating execution, and this would only be executed for some clients, leading to buggy code.

function doWork(deadline) {
  var iterations = 10;
  while (deadline.timeRemaining === undefined ? iterations-- : deadline.timeRemaining > 0) 
    doSomethingExpensive();
}

If you had a timeRemaining() or isExceeded() method you could easily build a mock to pass into the doWork method. This would be useful for frameworks or libraries that do their own scheduling.

 class FakeIdleDeadline {
    constructor(iterations) { this.iterations = iterations }
    timeRemaining() { return this.iterations--; }
 }   
 doWork(new FakeIdleDeadline(10));

Or, if you only cared about millisecond granularity you could use Date.now().

rmcilroy commented 9 years ago

Ok sounds reasonable. I've uploaded a change to the spec in pull request #14. PTAL, thanks.

igrigorik commented 9 years ago

@joshduck resolved in https://github.com/w3c/requestidlecallback/pull/14 - thanks for your feedback.