Ping?

Ping? This change is relatively short and passes all tests (including a new test). Please let me know if there's anything I can do to make this change easier to accept.

We're currently finishing up work on version 2.1. Afterwards, we'll be able to go through the waiting issues and pull requests.

Great, thanks for the update.

I haven't yet looked out your code in detail, but this will be on our list of items for version 2.2. Other possible implementations include @sciolizer's De-duping calls to evaluateImmediate, or parts of my deferred updates plugin.

Great! I'm happy with anything that passes my test case with good efficiency. If you do go with either of the other solutions, please do test it with my test case.

Needs to be reviewed in detail for any possible side-effects, but in principle, this sounds like a worthy improvement. I'd be pleased to have it in 2.2.

Thanks Haberman!

Hi there, it looks like you haven't gotten a chance to fold this change into knockout? I tried my test case with the latest repo and it is still failing with stack overflow.

Anything I can do to help this change get accepted?

Thanks,
Josh

Here's my version of this that I just added to my Deferred Updates plugin:

var oldnotifySubscribers = ko.subscribable.fn.notifySubscribers, notifyQueue;
ko.subscribable.fn.notifySubscribers = function (valueToNotify, event) {
    if (event === 'change' || event === 'dirty' || event === undefined) {
        if (!notifyQueue) {
            try {
                notifyQueue = [];
                oldnotifySubscribers.call(this, valueToNotify, event);
                if (notifyQueue.length) {
                    for (var i = 0, n; n = notifyQueue[i]; i++) {
                        oldnotifySubscribers.call(n.object, n.value, n.event);
                    }
                }
            } finally {
                notifyQueue = null;
            }
        } else {
            notifyQueue.push({
                object: this,
                value: valueToNotify,
                event: event
            });
        }
    } else {
        oldnotifySubscribers.call(this, valueToNotify, event);
    }
};

The code I attached above for notifySubscribers uses a queue/loop instead of recursion. It can be used in any application that runs into "too much recursion" errors. I'm hesitant to make this method the default behavior in Knockout, though, because it's more complicated than recursion and makes it hard to debug dependencies.

As a compromise, I've come up with some changes (#1088) that provide a 100%+ increase in the maximum dependency depth.

@haberman – If you have a look at the comments linked to this issue, you'll note that the plan is, at the moment, to have this problem persist on legacy Firefox, for two reasons:

1. One can work around this with deferred updates, which is available as a plugin for KO 3.3 and before, and integrated as of (forthcoming) 3.4
2. The problem only exhibits on old versions of Firefox (versions 10 to 23)

The patch you proposed certainly looks to work, but it adds some complexity and does not extend to other event types that may also be affected.

On that basis I'll close this issue, but please comment if you think we're missing something important here and we can certainly discuss/re-open.

Sorry it took so long to get to this @haberman . I hope you found a workaround in the mean time!

Cheers