Skip to content
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

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

Open
tkriplean opened this issue Apr 23, 2016 · 1 comment

Comments

@tkriplean
Copy link
Member

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
Copy link
Member

toomim commented Apr 24, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants