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.

Sign up for GitHub

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

Jump to bottom

StoreAdapter does not guarantee that fetch, etc. return a promise #124

Closed
kfranqueiro opened this issue May 27, 2015 · 1 comment
Closed

StoreAdapter does not guarantee that fetch, etc. return a promise #124

kfranqueiro opened this issue May 27, 2015 · 1 comment
Assignees
@maier49
Labels
bug

Comments

@kfranqueiro
Copy link
Contributor

The following simple test case fails with an error on dgrid's dev-0.4 and master branches (which have removed what should be excessive when calls), because the adapter returns query results with immediate values instead of a promise:

<!DOCTYPE html>
<html>
    <body>
        <div id="grid"></div>
        <script src="../../dojo/dojo.js" data-dojo-config="async: true"></script>
        <script>
            require([
                'dojo/_base/declare',
                'dojo/on',
                'dgrid/OnDemandGrid',
                'dojo/store/Memory',
                'dstore/legacy/StoreAdapter',
            ], function (declare, on, OnDemandGrid, Memory, StoreAdapter) {
                var legacyStore = new Memory({ data: [
                    { id: 1, name: 'foo' },
                    { id: 2, name: 'bar' }
                ] });
                var store = new StoreAdapter({ objectStore: legacyStore });

                var grid = new OnDemandGrid({
                    columns: { id: 'ID' },
                    collection: store
                }, 'grid');
            });
        </script>
    </body>
</html>
@kfranqueiro
Copy link
Contributor Author

as @maier49 has started looking at this, it quickly occurred to us that a bunch of StoreAdapter tests need updating regarding this expectation as well.

It actually makes me wonder if we should consider fixing this to be a break in backwards compatibility, but technically the original behavior was inconsistent and incorrect.

maier49 added a commit to maier49/dstore that referenced this issue Jun 30, 2015
@maier49
Ensure that StoreAdapter always returns a promise for 'fetch', etc.
8400b69 
Fixes SitePen#124 by wrapping the return value from StoreAdapter's
fetchRange function in a promise. This function is delegated
to by fetch so this handles both cases. Also updated several
test cases that were relying on the synchronous behavior of
the StoreAdapter.
@maier49 maier49 mentioned this issue Jun 30, 2015
Closed
kfranqueiro pushed a commit that referenced this issue Jul 1, 2015
@maier49
Ensure that StoreAdapter always returns a promise for 'fetch', etc.
b74d32a 
Fixes #124 by wrapping the return value from StoreAdapter's
fetchRange function in a promise. This function is delegated
to by fetch so this handles both cases. Also updated several
test cases that were relying on the synchronous behavior of
the StoreAdapter.

(cherry picked from commit 34b51c9)
kfranqueiro pushed a commit that referenced this issue Jul 1, 2015
@maier49
Ensure that StoreAdapter always returns a promise for 'fetch', etc.
34b51c9 
Fixes #124 by wrapping the return value from StoreAdapter's
fetchRange function in a promise. This function is delegated
to by fetch so this handles both cases. Also updated several
test cases that were relying on the synchronous behavior of
the StoreAdapter.

(Analogous to b7fb8b9 on master)
kfranqueiro pushed a commit that referenced this issue Jul 1, 2015
@maier49
Ensure that StoreAdapter always returns a promise for 'fetch', etc.
18c56ba 
Fixes #124 by wrapping the return value from StoreAdapter's
fetchRange function in a promise. This function is delegated
to by fetch so this handles both cases. Also updated several
test cases that were relying on the synchronous behavior of
the StoreAdapter.

(cherry picked from commit 34b51c9)
maier49 added a commit to maier49/dstore that referenced this issue Jul 10, 2015
@maier49
Ensure that StoreAdapter always returns a promise for 'fetch', etc.
ad51b68 
Fixes SitePen#124 by wrapping the return value from StoreAdapter's
fetchRange function in a promise. This function is delegated
to by fetch so this handles both cases. Also updated several
test cases that were relying on the synchronous behavior of
the StoreAdapter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maier49 maier49

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kfranqueiro @maier49