derbyjs / racer

Realtime model synchronization engine for Node.js
1.19k stars 118 forks source link

model.filter() & model.sort() don't work without specifying a filter/sort function #203

Closed dadisn closed 9 years ago

dadisn commented 9 years ago

The parseFilterArguments() function that parses arguments passed to model.filter() and model.sort() does not correctly parse calls to these functions when a input path is the only argument. Instead, parseFilterArguments() takes the input path provided and assumes it is a filter function. When this turns out to be inaccurate, Filter.filter() and Filter.sort() throw an error:

model.filter('collection');
Uncaught TypeError: Filter function not found: collection

model.sort('collection')
Uncaught TypeError: Sort function not found: collection 

The same issue also presents itself when the filter/sort function is omitted but other arguments are provided to the filter/sort (i.e. options object, other input paths).

Previously, it has been possible to create a filter/sort without passing a filter/sort function, thereby passing everything through, in case of a filter, or sorting according to sort function asc, in case of sort. This behaviour is also described in the docs that are now up on http://derbyjs.com/docs/derby-0.6/models/filters-and-sorts. Judging by the code and the docs, it seems likely that this is to be supported. However, if not, then the docs could just be updated to represent this since the problem can easily be circumvented by passing an undefined instead of the filter function.

minicuper commented 9 years ago

I think recent commit broke the feature - https://github.com/derbyjs/racer/commit/a87f347fc1c90c0de1f3317342ad419b08f1a5fd

dadisn commented 9 years ago

The docs have been updated to reflect the current functionality of model.filter() and model.sort(), rendering this issue irrelevant.

nateps commented 9 years ago

Yeah, you just have to provide a function parameter right now. It is too imprecise to distinguish between strings that could mean model paths and strings that could be the name of a function. The argument parsing is now the same as it is for model functions, since there are a variable number of arguments now. You can still pass null for the last argument if you don't want to pass a function.