Opened 11 years ago

Closed 6 years ago

Last modified 6 years ago

#8306 closed defect (wontfix)

Tree: destroy() does not cancel request

Reported by: Les Owned by: bill
Priority: low Milestone: 1.10
Component: Dijit Version: 1.2.3
Keywords: Tree Cc:
Blocked By: Blocking:

Description

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

Change History (16)

comment:1 Changed 11 years ago by bill

Summary: Tree.destroy() does not cancel requestTree: destroy() does not cancel request

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?

comment:2 in reply to:  1 Changed 11 years ago by Les

Replying to bill:

BTW which data store are you using?

I use dojo.data.ItemFileReadStore?

comment:3 Changed 11 years ago by 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

comment:4 Changed 11 years ago by bill

  1. 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();
}

comment:5 Changed 11 years ago by bill

Milestone: 1.31.4
Owner: set to bill

comment:6 Changed 11 years ago by 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.

comment:7 Changed 10 years ago by bill

Milestone: 1.4future

comment:8 Changed 10 years ago by 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

comment:9 Changed 10 years ago by bill

Milestone: future2.0

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.

comment:10 Changed 7 years ago by bill

Priority: highlow

comment:11 Changed 7 years ago by bill

Milestone: 2.01.10

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

comment:12 Changed 6 years ago by ben hockey

Milestone: 1.102.0

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.

comment:13 Changed 6 years ago by bill

Milestone: 2.01.10

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.

comment:14 Changed 6 years ago by bill

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

Last edited 6 years ago by bill (previous) (diff)

comment:15 Changed 6 years ago by bill

Resolution: wontfix
Status: newclosed

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.

comment:16 Changed 6 years ago by Bill Keese <bill@…>

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? 
Note: See TracTickets for help on using tickets.