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

async mayHaveChildren support in Tree.js #115

Closed
ozdev opened this Issue Mar 11, 2012 · 7 comments

Comments

Projects
None yet
4 participants
@ozdev

ozdev commented Mar 11, 2012

It'll be nice if store.mayHaveChildren() in Tree.js can return a Deferred like other methods. I've experimented with the idea by wrapping the part after getting mayHaveChildren in Deferred.when() and it seemed to be OK.

@kriszyp

This comment has been minimized.

Contributor

kriszyp commented Mar 12, 2012

The point of mayHaveChildren is to be able to quickly and synchronously determine which nodes might have children with requiring a full asynchronous request/operation for their children. What is the use case for needing an asynchronous mayHaveChildren?

@ozdev

This comment has been minimized.

ozdev commented Mar 13, 2012

In cases where asynchronous requests for children can be done locally (by using IndexedDB for example) and/or are cheap enough, an asynchronous mayHaveChildren frees you from having to create a property like 'childrenCount' or load all the data and cache them beforehand just to prevent expando icons from showing up next to nodes with no children.

@kfranqueiro

This comment has been minimized.

Contributor

kfranqueiro commented Mar 13, 2012

On the other hand, you also have to realize that mayHaveChildren is called for every cell in the tree column, which means you could easily be issuing upwards of 20 calls at once. Moreover, when the user actually does expand the child, that will then issue its own request. Asynchronous mayHaveChildren doesn't seem advisable, compared to the pattern of simply including a tiny bit of metadata with each item in the query result itself. (Moreover, I wouldn't want to entice people to toss an entire server request in there, which would be rather terrible.)

It also may be worth pointing out that the mayHaveChildren concept originated with dijit/Tree models, which only support synchronous implementations of the method.

@ozdev

This comment has been minimized.

ozdev commented Mar 16, 2012

This old ticket well describes the problem I want to solve by asynchronous mayHaveChildren. Sometimes it's simply impossible to know whether a given node has children or not until actually requesting for it. So, I think, supporting both synchronous and asynchronous mayHaveChildren at least gives developers a choice whether to solve that 'empty folder' problem at all costs or just live with that.

Well, that's all I can say on this. So, feel free to close this issue if others think there is no need to discuss further.

@mikerobi

This comment has been minimized.

mikerobi commented May 4, 2012

The point of mayHaveChildren is to be able to quickly and synchronously determine ...

Shouldn't the point be "to quickly determine?" Asynchronously does not automatically mean slowly. Shouldn't that determination be up to the developer?

I propose a compromise:

Add a prefetch property to the store. If it is true, mayHaveChildren is not called. Instead, expanding a row causes getChildren to be called for each child.

prefetch makes it pretty clear that you are doing something that could add significant overhead.

@ghost

This comment has been minimized.

ghost commented May 24, 2012

@mikerobi, I may be misreading your suggestion, I'm kind of confused as to how what you're suggesting is a compromise. Expanding a row is already what causes getChildren to be called; mayHaveChildren is simply called up-front to check whether an expando should even be displayed whatsoever for each row. If no mayHaveChildren method is defined on the store, every row will have an expando unconditionally.

If you're suggesting to call getChildren recursively on all children the moment a root-level row is opened, I'm not really sure how that solves the OP's problem, and I'd be rather reluctant to go that route. (Incidentally, the shouldExpand logic in #172 should allow for that if people are bent on it.)

@kfranqueiro

This comment has been minimized.

Contributor

kfranqueiro commented Jan 29, 2015

mayHaveChildren is still intended to be a synchronous API even in dstore. Given that, it doesn't make much sense to add complexity to dgrid for an unintended use of the API. (Again, if you omit mayHaveChildren entirely, you will just end up seeing expand icons on every row.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment