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

todo page to index page to todos page, app should not download JSON data again #6

Open
xqiu opened this issue Apr 19, 2013 · 6 comments

Comments

@xqiu
Copy link
Owner

xqiu commented Apr 19, 2013

From comment by majco333 in http://www.asp.net/single-page-application/overview/templates/emberjs-template

Try change index page and back to todos page - your app download JSON data again.
You should not store data in the store? How do this?

@MilkyWayJoe
Copy link
Contributor

Reproduced. This seems to be related with the primary key. I'm having the same issue when using the WebAPIAdapter & WebAPISerializer. In my tests I could not reproduce the issue when using the RESTAdapter and RESTSerializer in Rails, but in this scenario I don't replace the primary key.

The Data Store expect the Model to have an id attribute which is the primary key, and since the configuration sets it to todoItemId it seems like the app can't find the items which are already in store and fetches data from the server again, replacing them.

I'm still not sure whether this is related to the adapter and serializer only, if it's a bug within Ember or a combination of both; but it seems like it could be the configuration for the Web API replacing the PK.

One alternative solution would be to get rid of the WebAPIAdapter and WebAPISerializer and add custom a JSON serializer extending JSON.NET so instead of camelCase the API returns snake_case payload, then use the default adapter and serializer.

@xqiu
Copy link
Owner Author

xqiu commented Apr 26, 2013

It seems that I can solve the issue really easily by modifying TodoListRoute.js file:

App.todoListContent = null;
App.TodoListRoute = Ember.Route.extend({
    setupController: function (controller, model) {
        if (App.todoListContent == null) {
            App.todoListContent = App.TodoList.find();
        }
        controller.set('content', App.todoListContent);
    }
});

Do you see any problem with the code above? Is it the right pattern?

@MilkyWayJoe
Copy link
Contributor

I don't think this is it. It may stop the app from calling the backend API every time but in the wrong place; and I think this would essentially do a fraction I'd what the store does. It should be done in the adapter not in the route, and definitely not up for the developer.
When I was debugging noticed it finds the records in the store cache before it sends a request. It might be related to changes made from revision 11 to 12 that may not be applied, but I still suspect it also could be something related to the primary key.

@MilkyWayJoe
Copy link
Contributor

The more I look at it, the more I get convinced it's related to the DS.WebAPIAdapter

@xqiu
Copy link
Owner Author

xqiu commented May 4, 2013

Here's the comments from the ember.js about the caching and state. Looks like it recommended we check the cache and reload ourselves.

The `find` method will always return the same object for a given type and
`id`. To check whether the adapter has populated a record, you can check
its `isLoaded` property.

---

To find all records for a type, call `find` with no additional parameters:

    store.find(App.Person);
    App.Person.find();

This will return a `RecordArray` representing all known records for the
given type and kick off a request to the adapter's `findAll` method to load
any additional records for the type.

The `RecordArray` returned by `find()` is live. If any more records for the
type are added at a later time through any mechanism, it will automatically
update to reflect the change.

---

To find a record by a query, call `find` with a hash as the second
parameter:

    store.find(App.Person, { page: 1 });
    App.Person.find({ page: 1 });

This will return a `RecordArray` immediately, but it will always be an
empty `RecordArray` at first. It will call the adapter's `findQuery`
method, which will populate the `RecordArray` once the server has returned
results.

You can check whether a query results `RecordArray` has loaded by checking
its `isLoaded` property.

The funny thing is that it's not easy to check isLoaded for the array. through the functions. Apparently ember.data still improving on the related functions.

For example, the following doesn't work as App.TodoList.isLoaded() always return false due to it needs an id as argument.

App.TodoListRoute = Ember.Route.extend({
    setupController: function (controller, model) {
        if (!App.TodoList.isLoaded()) {
            controller.set('content', App.TodoList.find());
        }
    }
});

The version we have in ember-data.js defines:

    recordIsLoaded: function (type, id) {
        return !Ember.isNone(this.typeMapFor(type).idToCid[id]);
    },

I debugged again for App.TodoList.find() by tracing through it, it doesn't seem to check for any isLoaded state before querying for the AJAX result, so it does seem we need to check the cache ourselves.

So maybe I should submit a bug for isLoaded function to improve it for id as undefined case to improve it, and we use the following in the route?
if (!App.TodoList.isLoaded()) {
controller.set('content', App.TodoList.find());
}

@MilkyWayJoe
Copy link
Contributor

There's something else. I think we should wait a bit because ember-data is close to another revision, and there will be significant changes that's worth to wait until they are in place to fix the adapter. I don't think this is something that should be a concern of the application, should not be the developer's concern to apply a workaround to fix the framework for a given scenario, that's what the adapter is for.

I've created a new project only for the adapter, I'll send you ownership/invitation sometime tomorrow. I think the adapter has to be changed by the next version of ember data (either revision 13 or in 1.0) and we test the hell out of it before adding into nuget.

Right not it's just the exact same serializer/adapter you have, but I'll be working on it starting tomorrow
https://github.com/MilkyWayJoe/Ember-WebAPI-Adapter
Then I'll add into nuget and update the template reference

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