Allow extend to change private methods like runTask

I wanted to extend fastdom to process tasks in a different way. Specifically I wanted to use the advice from https://developers.google.com/web/fundamentals/performance/rendering/optimize-javascript-execution to break up the tasks so only a few ran per frame. Currently fastdom runs all the tasks in a flush. So if you have queued up a bunch of tasks you still can get a javascript violation warning if the code ran for too long.

The google link provides an example like:

var taskList = breakBigTaskIntoMicroTasks(monsterTaskList);
requestAnimationFrame(processTaskList);

function processTaskList(taskStartTime) {
  var taskFinishTime;

  do {
    // Assume the next task is pushed onto a stack.
    var nextTask = taskList.pop();

    // Process nextTask.
    processTask(nextTask);

    // Go again if there’s enough time to do the next task.
    taskFinishTime = window.performance.now();
  } while (taskFinishTime - taskStartTime < 3);

  if (taskList.length > 0)
    requestAnimationFrame(processTaskList);

}

I've managed to modify fastdom to incorporate that but would prefer to do it in an extension rather then modifying the main.

This was my first attempt at putting it into fastdom (only the changed methods):

    function TimeExceededException() { }

    /**
     * Runs queued `read` and `write` tasks.
     *
     * Errors are caught and thrown by default.
     * If a `.catch` function has been defined
     * it is called instead.
     *
     * @private
     */
    function flush(fastdom, taskStartTime) {
        debug('flush');
        var writes = fastdom.writes;
        var reads = fastdom.reads;
        var error;
        try {
            debug('flushing reads', reads.length);
            runTasks(reads, taskStartTime);
            debug('flushing writes', writes.length);
            runTasks(writes, taskStartTime);
        }
        catch (e) {
            error = e;
        }
        fastdom.scheduled = false;
        // If the batch errored we may still have tasks queued
        if (reads.length || writes.length)
            scheduleFlush(fastdom);
        if (error && !(error instanceof TimeExceededException)) {
            debug('task errored', error.message);
            if (fastdom["catch"])
                fastdom["catch"](error);
            else
                throw error;
        }
    }
    /**
     * We run this inside a try catch
     * so that if any jobs error, we
     * are able to recover and continue
     * to flush the batch until it's empty.
     *
     * @private
     */
    function runTasks(tasks, taskStartTime) {
        debug('run tasks');
        var task;
        var taskFinishTime = window.performance.now();
        while ((taskFinishTime - taskStartTime < 3) && (task = tasks.shift())) {
            task();
            // Go again if there’s enough time to do the next task.
            taskFinishTime = window.performance.now();
        }
        if (tasks.length) {
            throw new TimeExceededException();
        }
    }

Basically it processes the task, measuring how much time has passed and if it hasn't been too long keeps processing the tasks. If no time left it throws an exception which is caught and ignored, but used to queue up for a next round.

Might be a better way that doesn't involve throwing exceptions (I'm a pythonista, exceptions are the norm). But I don't see any way to currently use extend to modify fastdom for this behaviour.

There was a PR in the past that did something similar. Although conceptually I think this is cool, I pushed back on it being in the core as I feared it could lead to in consistent behaviour across devices of varying abilities.

For example, say your using FastDOM to construct a complex layout, on a fast device all the work may get done in one frame, but on slower devices it may get spread across frames, resulting in a more 'progressive' style render.

I'm willing to be proven wrong though :)

To your earlier point of allowing extensions to override the core task flush logic: this seems fragile to me. IMO how fastdom chooses to run the tasks should be abstracted from user/extension land. I say this as I think we'll probably break extensions if we choose/find new optimizations further down the road.

I think the best solution in this case would be to add this particular logic to the core, but preffed off by default. This means that users can opt-in to more unpredictable running of tasks in exchange for some smart flushing and we can keep the task flushing logic private from extensions.

The downside is that we'll bloat the core a little. Right now my suggestion would be for you to run your fork in production for a while and report back any issues that arise. If the patch proves to be truly valuable we can merge it into the core.

Thanks for your time on this, it's really interesting. Hope my feedback helps :)

wilsonpage / fastdom

Allow extend to change private methods like runTask #106