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

orderBy should work for ngRepeat over objects too #1286

Closed
andrewrk opened this issue Aug 22, 2012 · 78 comments
Closed

orderBy should work for ngRepeat over objects too #1286

andrewrk opened this issue Aug 22, 2012 · 78 comments

Comments

@andrewrk
Copy link

@andrewrk andrewrk commented Aug 22, 2012

http://jsfiddle.net/m7ynD/

The dates are not sorted, but they should be.

@btford
Copy link
Contributor

@btford btford commented Aug 23, 2012

I'm not convinced that you should be able to sort objects with a filter; if the order of some collection of models is important, it seems like you would want to use an array, or augment your model with an array with references to the items in the collection. Can you provide some real-world example of why this would be desirable?

@andrewrk
Copy link
Author

@andrewrk andrewrk commented Aug 26, 2012

Here's the real world example. My model is a bunch of jobs to process. A job looks like this:

Job model
{
    state: // one of ['queued', 'processing', 'complete']
    type: // one of ['audio', 'image']
    id: // some unique string
    queue_date: // date that the job was queued
}

jobs_by_id model
{
    <job id>: <job>
}

jobs_by_category model
{
    queued: {
        audio: {
            <job id>: <job>
        }
        image: {
            <job id>: <job>
        }
    }
    processing: {
        audio: {
            <job id>: <job>
        }
        image: {
            <job id>: <job>
        }
    }
    complete: {
        audio: {
            <job id>: <job>
        }
        image: {
            <job id>: <job>
        }
    }

The view looks something like this:

<h2>Jobs</h2>
<h3>Queued</h3>
<div job-table="queued"></div>
<h3>Processing</h3>
<div job-table="processing"></div>
<h3>Complete</h3>
<div job-table="complete"></div>

The job-table directive looks like this:

main.directive('jobTable', function(){
  return {
    templateUrl: '/views/job-table.html',
    scope: true,
    link: function(scope, element, attrs){
      scope.state = attrs.jobTable;
      scope.state_jobs = scope.jobs_by_category[scope.state];
    }
  };
});

The job-table.html view looks something like this:

<h4>audio</h4>
<table>
    <tr>
        <th>date job was queued</th>
        (a bunch more <th>'s related to audio)
    </tr>
    <tr ng-repeat="job in state_jobs.audio">
        <td>{{job.queue_date}}</td>
        (a bunch more <td>'s related to audio)
    </tr>
</table>
<h4>image</h4>
<table>
    <tr>
        <th>date job was queued</th>
        (a bunch more <th>'s related to image)
    </tr>
    <tr ng-repeat="job in state_jobs.image">
        <td>{{job.queue_date}}</td>
        (a bunch more <td>'s related to image)
    </tr>
</table>

I need the job <tr>s to be sorted by job.queue_date. You may say that I should
use arrays instead of objects for jobs_by_category, but I need the data
structure to be the way it is for inserting and deleting jobs to make sense.
Here's what the insert/delete job code looks like:

function insertJobIntoCat(job){
  var ref$, key$;
  ((ref$ = $scope.jobs_by_cat[job.state])[key$ = job.type] || (ref$[key$] = {}))[job.id] = job;
}
function newJob(job){
  jobs_by_id[job.id] = job;
  insertJobIntoCat(job);
}
function updateJob(job){
  var old_job, ref$, key$;
  old_job = jobs_by_id[job.id];
  if (old_job.state === job.state && old_job.type === job.type) {
    import$(old_job, job);
  } else {
    delete ((ref$ = $scope.jobs_by_cat[old_job.state])[key$ = old_job.type] || (ref$[key$] = {}))[old_job.id];
    insertJobIntoCat(import$(old_job, job));
  }
}
function import$(obj, src){
  var own = {}.hasOwnProperty;
  for (var key in src) if (own.call(src, key)) obj[key] = src[key];
  return obj;
}

So as you can see, if I wanted to switch to using an array in the
jobs_by_category model, I would have to give up O(1) insertion and
deletion of jobs and settle with O(n). Even though the performance of
this particular thing isn't an issue, it's the principle of the matter.

The orderBy filter should be able to sort objects. It would not confuse the
current syntax, and it would solve this perfectly valid problem that I am
having.

@Primusio
Copy link

@Primusio Primusio commented Sep 4, 2012

I would also appreciate this feature.

@btford
Copy link
Contributor

@btford btford commented Sep 4, 2012

@superjoe30, thanks for the extensive example! I've been busy the past week, but I'm still looking over this and thinking about it. I like the semantics, but implementation would be complicated, and I think it's more semantic to use an array when order matters.

Complexity analysis for this isn't as straightforward as what you've suggested; although you can look at the insertions/deletions in isolation, the reality of the situation is that you also have to do dirty checking and updating of the DOM based on the changing data structure, and those operations are likely to be much more expensive, perhaps to the extent that O(N) insertion/deletion is negligible. We'd need to benchmark this to get a real answer.

@seamusmalone
Copy link

@seamusmalone seamusmalone commented Oct 23, 2012

I third the request. Its really inefficient and risks denormalization to shift objects into arrays just to sort them in some view

@brev
Copy link

@brev brev commented Oct 25, 2012

Made a few small changes to the orderByFilter code to allow objects, instead of just arrays:

https://gist.github.com/3949705

@tmaiaroto
Copy link

@tmaiaroto tmaiaroto commented Nov 14, 2012

Me too...I simply render JSON after a database query from MongoDB. That returns objects. Even if it didn't, the classes within the framework I'm using returns objects when converting to JSON because the document or record class is managing the data model. In many, many cases this will be true across other web applications and APIs.

I really think this is a deficiency with JSON "objects" that can be resolved by Angular. Ensuring these objects are processed in the "proper" order has always been an issue. So people make methods just for this purpose or output two hacky arrays sometimes or just always output arrays in general. It's very convenient to use JSON objects. I don't know, blame the JSON spec for how we got here...But we're here.

@BernhardPosselt
Copy link

@BernhardPosselt BernhardPosselt commented Nov 29, 2012

Very interested in this, already added this in my version. Could somebody add this?

@sssilver
Copy link

@sssilver sssilver commented Dec 6, 2012

+1

3 similar comments
@sitamet
Copy link

@sitamet sitamet commented Dec 8, 2012

+1

@Yukilas
Copy link

@Yukilas Yukilas commented Dec 27, 2012

+1

@aaronshaf
Copy link

@aaronshaf aaronshaf commented Jan 7, 2013

+1

@andrewrk
Copy link
Author

@andrewrk andrewrk commented Jan 16, 2013

The real world example I gave above is now open source.

@klebba
Copy link

@klebba klebba commented Jan 28, 2013

+1

7 similar comments
@brotherdetjr
Copy link

@brotherdetjr brotherdetjr commented Feb 10, 2013

+1

@moroshko
Copy link
Contributor

@moroshko moroshko commented Feb 19, 2013

👍

@mlarcher
Copy link
Contributor

@mlarcher mlarcher commented Feb 19, 2013

+1

@toxaq
Copy link

@toxaq toxaq commented Feb 21, 2013

+1

@agrublev
Copy link

@agrublev agrublev commented Feb 24, 2013

+1

@wookiesh
Copy link

@wookiesh wookiesh commented Feb 26, 2013

+1

@mattkennedy
Copy link

@mattkennedy mattkennedy commented Feb 26, 2013

+1

@slundberg
Copy link

@slundberg slundberg commented Mar 1, 2013

The solution by @brev works great if you don't mind modifying angular and don't care about your keys, since they get replaced by indexes. However, I wanted to have access to my keys while also sorting and searching. I also did not want to create an array version of my object on my scope that I had to keep in sync during changes.

As @btford mentioned it is not trivial to do, the orderBy filter must return an array since there is no way to "order" an object's properties. This means that for this to "just work", ngRepeat would have to recognize a special kind of array (that saves key information) as derived from an object and then bind saved keys to the (key,value) paring instead of indexes to (index,value) as would normally occur with arrays.

In order to avoid making changes to angular, a simple solution is to use another filter. This filter converts a general object to an array and saves the key away for later use as a non-enumerable property.

The filter (not perf tuned and using underscore):

app.filter('toArray', function() { return function(obj) {
    if (!(obj instanceof Object)) return obj;
    return _.map(obj, function(val, key) {
        return Object.defineProperty(val, '$key', {__proto__: null, value: key});
    });
}});

Example markup:

<div ng-repeat="val in object | toArray | orderBy:'priority' | filter:fieldSearch">
  {{val.$key}} : {{val}} 
</div>

Potential downsides:

  • Object.defineProperty will not work like this in IE8 (if you don't mind making $key enumerable then this is an easy change).
  • If your property values are not objects then you can't set the $key property and this fails.
  • It is not directly integrated into orderBy or ngRepeat so the syntax is different.

Integrating directly into ngRepeat and orderBy/filter could address these issues but they didn't impact my project.

@AndreasHeintze
Copy link

@AndreasHeintze AndreasHeintze commented Apr 2, 2013

I didn't quite understand how to make your filter "toArray" work in IE8, can you explain?

@slundberg
Copy link

@slundberg slundberg commented Apr 2, 2013

Object.defineProperty creates a property that is not enumerable. See http://kangax.github.com/es5-compat-table/ for a note on IE8 compatibility where it states that Object.defineProperty only works on DOM objects in IE8. A simple change is to replace return Object.defineProperty(val, '$key', {__proto__: null, value: key}); with val.$key = key; return val;

@drawyan
Copy link

@drawyan drawyan commented Apr 24, 2013

+1, and @slundberg it's a great idea just temporarily converting object to an array, thanks.

@misel228
Copy link

@misel228 misel228 commented Apr 26, 2013

+1

3 similar comments
@Jenk-Rendar
Copy link

@Jenk-Rendar Jenk-Rendar commented May 29, 2013

+1

@wildermuthn
Copy link

@wildermuthn wildermuthn commented Jun 5, 2013

+1

@sirMackk
Copy link

@sirMackk sirMackk commented Jun 7, 2013

+1

@vendethiel
Copy link

@vendethiel vendethiel commented Jun 7, 2013

This is open source software. The best way to get progress you want is to send me a patch. Comments that just say +1 are the GitHub version of this:

@realyze
Copy link

@realyze realyze commented Mar 26, 2014

+1

3 similar comments
@ghost
Copy link

@ghost ghost commented Mar 28, 2014

+1

@sgmonda
Copy link

@sgmonda sgmonda commented Apr 2, 2014

+1

@ajohnclark
Copy link

@ajohnclark ajohnclark commented Apr 21, 2014

+1

anglee added a commit to twosigma/beakerx that referenced this issue May 2, 2014
Because at the moment sorting(orderBy property) object is not well supported. angular/angular.js#1286
@darklrd
Copy link

@darklrd darklrd commented May 7, 2014

+1

12 similar comments
@oknoorap
Copy link

@oknoorap oknoorap commented May 10, 2014

+1

@jiteshgolecha
Copy link

@jiteshgolecha jiteshgolecha commented May 23, 2014

+1

@eugef
Copy link

@eugef eugef commented May 27, 2014

+1

@kinoakter
Copy link

@kinoakter kinoakter commented Jun 4, 2014

+1

@catamphetamine
Copy link

@catamphetamine catamphetamine commented Jun 4, 2014

+1

@tfrere
Copy link

@tfrere tfrere commented Jun 15, 2014

+1

@Londeren
Copy link

@Londeren Londeren commented Jun 18, 2014

+1

@hilts-vaughan
Copy link

@hilts-vaughan hilts-vaughan commented Jun 26, 2014

+1

@nidDrBiglr
Copy link

@nidDrBiglr nidDrBiglr commented Jul 1, 2014

+1

@lasfin
Copy link

@lasfin lasfin commented Jul 9, 2014

+1

@Sergey6116
Copy link

@Sergey6116 Sergey6116 commented Jul 22, 2014

+1

@Fonger
Copy link

@Fonger Fonger commented Jul 25, 2014

+1

@vs4vijay
Copy link

@vs4vijay vs4vijay commented Jul 31, 2014

How to sort date using this?

@nodecode
Copy link

@nodecode nodecode commented Aug 1, 2014

I'm currently developing a real-time browser game with AngularJS, in which I have a room list. To update individual entries in this list, the server sends a notification with the new version of this single entry when it changes. Thus, a single entry can be by ID, provided by the server, updated, I need to use an object instead of an array.

I would like to sort these entries with AngularJS, which, however, currently not working. I would be happy if this feature will be implemented soon. Otherwise, I had to write such a function myself, which I however come back in the boilerplate code hell.

@petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented Aug 3, 2014

Here is a hard-coded solution to the OP: http://jsfiddle.net/q48Vt/
Here is a version that uses @finalclass's toArray filter (remembering that this will not work on IE8): http://jsfiddle.net/z8ME5/

Here is a version that uses toArray but actually modifies the original object (and will work on IE8):
http://jsfiddle.net/JHL6X/

@petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented Aug 4, 2014

I have created a module that provides the toArray filter for helping with these situations

http://ngmodules.org/modules/angular-toArrayFilter

@raoulus
Copy link

@raoulus raoulus commented Oct 24, 2014

+1

2 similar comments
@cef62
Copy link

@cef62 cef62 commented Dec 31, 2014

+1

@2013gang
Copy link

@2013gang 2013gang commented Jan 9, 2015

+1

@angular angular locked and limited conversation to collaborators Jan 11, 2015
@petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented Jan 11, 2015

The 'toArray' filter provided above is a workable solution to this problem. Without completely rewriting how ngRepeat works, which is outside of the scope for 1.x, it is not feasible to solve this in any better way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.