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

Trackable does not preserve totalLength property on fetch results #97

Closed
kfranqueiro opened this issue Feb 14, 2015 · 2 comments
Closed

Comments

@kfranqueiro
Copy link
Member

At https://github.com/SitePen/dstore/blob/v1.0.1/Trackable.js#L98, the fetch function created by makeFetch returns a chained promise off of the original QueryResults object, losing any additional information such as totalLength. This will cause consumers that expect proper QueryResults to fail. (For example, this affects dgrid with Pagination and Tree, since it uses fetch for rendering children in that case.)

I've created a trackable-fetch-test branch with failing unit tests.

Meanwhile, fwiw, I suspect those when calls in Trackable's fetch/fetchRange functions can be simplified to use then now that the APIs are always async.

@kriszyp
Copy link
Member

kriszyp commented Mar 19, 2015

I am hesitant to backport this to 1.0.x since it slightly modifies the behavior of fetch, such that the array return from fetch (and fetchSync) is no longer updated/maintained by Trackable (it maintains a separate internal array). This is actually more consistent, as it matches the behavior of fetchRange, and I think more closely follows the intent of fetch, in that it returns a snapshot of data. However, this change in behavior seems like it would fit better in a 1.x release.

@kfranqueiro
Copy link
Member Author

If there's a clean workaround that you can suggest for people on 1.0.x, that might be good enough for now... otherwise, this issue can sort of throw a wrench in the works if you need to do something with totalLength on a tracked collection.

(Sorry, my initial response was totally thinking this was a different ticket...)

kriszyp added a commit that referenced this issue Mar 20, 2015
Preserves totalLength in a back-compat way for 1.0
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