mharris717 / ember-cli-pagination

Pagination Addon for Ember CLI
MIT License
272 stars 116 forks source link

Alias in controller of remote paginated scenario causes two network requests #214

Closed irruputuncu closed 7 years ago

irruputuncu commented 7 years ago

When implementing the remote paginated scenario and using query parameters as described, two requests are made due to the alias on the page attribute. However, when I would use oneWay instead of alias the query params are ignored.

This is how my controller looks atm (I only wanted the page query parameter, used model instead of content because of a deprecation warning and had to initialize the model for it to work..):

import Ember from 'ember';

export default Ember.Controller.extend({
  queryParams: ['page'],

  model: {},

  page: Ember.computed.alias('model.page')
});

The rest I implemented exactly as described in the readme.

balupton commented 7 years ago

This is how my controller looks atm (I only wanted the page query parameter, used model instead of content because of a deprecation warning and had to initialize the model for it to work..):

What was the code you use before to get the deprecation warning? and what was the deprecation warning?

balupton commented 7 years ago

Would you be able to setup a bare bones example that reproduces the double network request issue?

balupton commented 7 years ago

for those debugging, seems like issue would be found by backtracing from https://github.com/mharris717/ember-cli-pagination/blob/4be91f700e8358550a055241541ef50449df5424/addon/remote/paged-remote-array.js#L81-L133

irruputuncu commented 7 years ago

Basically like this (just as described in the readme):

import Ember from 'ember';

export default Ember.Controller.extend({
  queryParams: ['page'],

  content: {},

  page: Ember.computed.alias('content.page')
});

and the warning I got was DEPRECATION: Do not specify 'content' on a Controller, use 'model' instead. [deprecation id: ember-runtime.will-merge-mixin].

And if I would also left out the initialization of content I got this error: Property set failed: object in path "content" could not be found or was destroyed.

irruputuncu commented 7 years ago

Ok I found something: I don't know why exactly, but it seems like page and lastPage used for the comparison in line 111 of the paged-remote-array.js are not of the same type. page is a string and lastPage a number and thus page !== lastPage returns true and the content is fetched a second time. So using != solves it for me for now. (Tested in Chrome 56)

broerse commented 7 years ago

@irruputuncu Do you want to write a PR for this? I will merge it.

balupton commented 7 years ago

Makes you wish https://discuss.emberjs.com/t/ember-and-typescript/2898?u=balupton was a thing