#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 follow-up: 2 Changed 14 years ago by
Summary: | Tree.destroy() does not cancel request → Tree: destroy() does not cancel request |
---|
comment:2 Changed 14 years ago by
comment:3 Changed 14 years ago by
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 13 years ago by
- 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 13 years ago by
Milestone: | 1.3 → 1.4 |
---|---|
Owner: | set to bill |
comment:6 Changed 13 years ago by
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 13 years ago by
Milestone: | 1.4 → future |
---|
comment:8 Changed 13 years ago by
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.
comment:9 Changed 13 years ago by
Milestone: | future → 2.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 9 years ago by
Priority: | high → low |
---|
comment:11 Changed 9 years ago by
Milestone: | 2.0 → 1.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 8 years ago by
Milestone: | 1.10 → 2.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 8 years ago by
Milestone: | 2.0 → 1.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:15 Changed 8 years ago by
Resolution: | → wontfix |
---|---|
Status: | new → closed |
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.
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?