Tree: destroy() does not cancel request

[Les]

Request for tree data is not canceled when tree is destroyed.

http://dojotoolkit.org/forum/dijit-dijit-0-9/dijit-support/tree-model-how-cancel-request

[bill]

Looks like a destroy() / cancel() type method will have to be added to the model, which in turn will have to cancel any in-flight requests to the data store, either via fetch() or loadItem().

BTW which data store are you using?

[Les]

Replying to bill:

BTW which data store are you using?

I use dojo.data.ItemFileReadStore?

[bill]

OK... that loads all the data the first time you do any query, so it must be during that stage when you destroying the tree.... i.e., the initial load of the Tre

[bill]

5. Jared just checked in some fixes for #8315 to make it possible for Tree to cancel the request. I need to update TreeStoreModel/ForstStoreModel to saves the return value from fetch:

this._req = store.fetch()

And then in destroy() do:

//Check to see if we actually have an abort function
//Call it if we do.
if(this._req && this._req.abort){
        this._req.abort();
}

[bill]

Hmm this requires more changes than I thought. Destroying the tree doesn't destroy the model or the store, so what we need is a way for the Tree to cancel any pending requests that it has made to the model.

Thus, model.getRoot() and model.getChildren() need some way to be cancelled. Probably they should each be returning a Deferred Object, but I guess for backwards compatibility we can't depend on that.

TreeStoreModel.destroy() is also broken in that sense that it doesn't cancel any pending requests, and that needs to be fixed too, but it's not the cause of this bug.

[Les]

Is it now easier to provide this fix as a result the recent Tree changes for release 1.4?

Also, the latest Firebug (1.5b2) visually indicates aborted requests, which may be helpful in testing this fix.

​http://code.google.com/p/fbug/issues/detail?id=1526

[bill]

Well, it's still hard, I think it requires a change (an addition) to the model API which means it needs to be done in 2.0.

[bill]

Might as well check this into SVN and then merge to github, so it's available for the 1.x and 2.x streams.

[ben hockey]

breaking changes to the API would mean a 2.0 milestone - i can't tell why this was changed to 1.10. also, i don't see a patch so i'm not sure what is supposed to be merged here.

[bill]

breaking changes to the API would mean a 2.0 milestone

It could be done w/out breaking changes. Have getRoot() and getChildren() optionally return a (cancelable) Promise, and (if a promise was returned) have Tree.destroy() cancel it.

But perhaps it's easier to wait until 2.0, rather than adding more branching.

i don't see a patch so i'm not sure what is supposed to be merged here.

I was talking about merging the changes from Dijit V1 to V2, aka forward-porting.

[bill]

PS: this.own() was updated to support promises in #17680.

[bill]

Oh I didn't mean to set this back to 1.10. Actually, I'm just going to close this ticket and add a TODO to the code.

The main complication is that an ObjectStoreModel? can be shared between multiple trees, so there can be concurrent in-progress getChildren() calls, and destroying one tree should not interfere with an in-progress getChildren() call from another tree.

Combined with getChildren()'s caching of query results, this makes things complicated. Note that the caching is needed to prevent multiple calls to onChildrenChange() and onChange() for a single change to the data store.

[Bill Keese <[email protected]…>]

In fed794511f5c91d44875e0d02543503ae2a5f3af/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?