invisible-college / statebus

All aboard the STATEBUS!!!
118 stars 5 forks source link

component.loading never returns true if component fetches object returned nested on another object #16

Open tkriplean opened 8 years ago

tkriplean commented 8 years ago

pending_fetches isn't updated properly when you fetch an object with nested keyed objects.

Problems this causes

  1. my reactive functions with @loading guards never complete
  2. pending_fetches becomes a big memory hog when I have thousands of nested fetched objects

What is happening

Start at line 69 @ https://github.com/invisible-college/statebus/blob/master/statebus.js. There is a comment about counting keys that arrived on a nested object, with a subsequent call to bus.route.

Now imagine that we fetch an object that was nested when it arrived. Here's what happens:

  1. !fetches_out[key] returns true, so route is called with fetch and the key
  2. route calls run_handler
  3. run_handler adds the key to pending_fetches in the case of a fetch (line 487).

That key will never be removed from pending_fetches. The only time that keys are removed from pending_fetches is on pub, but pub won't be called for objects that arrived nested on another object.

What it looks like in my application

First, I modified statebus.js's loading_keys function to show which keys a function is waiting on, and whether that key is actually already in cache:

function loading_keys (keys) {
    // Do any of these keys have outstanding gets?
    // console.log('Loading: pending_keys is', pending_fetches)
    for (var i=0; i<keys.length; i++){
        if (pending_fetches[keys[i]]) {
            console.log("loading_keys: still loading", keys[i], fetch(keys[i]))
            return true
        }
    }
    return false
}

Second, I have a reactive function that fetches /strategies. By the time this reactive function is called, /strategies is already in cache because it was nested in a previously fetched object. This reactive function has an @loading guard.

Here's the console output:

screen shot 2016-04-23 at 11 18 52 am

Is that enough info, or do you need a minimal test case?

toomim commented 8 years ago

Yeah this bug report is too ambiguous. You wrote a lot of stuff, but a good bug report only needs to say:

  1. If you do this
  2. Then this happens
  3. Whereas this is what I expect to happen

You expressed (2) and (3) by saying that your reactive functions don't complete, but I don't know what your loading functions are, so I'm lost on (1).

A test case would help by providing (1) and (2), but it doesn't have to be minimal, and it's not the only way to do it. Making your testcase minimal is like a nice favor to do, because either of us could do that work of localizing the problem once we can reproduce it. But right now I can't reproduce the problem.