Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Unable to watch the changes of the property '$' of an object which is used by filterFilter. #13313

Closed
javeme opened this issue Nov 16, 2015 · 6 comments

Comments

@javeme
Copy link

javeme commented Nov 16, 2015

Unable to watch the changes of the property '$' of an object which is used by filterFilter.

//object to search 
$scope.search = {'$': 'something...'}

//watch the object
$scope.$watch('search',
  function (newValue, oldValue) {
    //do action 
    results = filter(list, search);
    ...
  }, true);

The "$" in the function filterFilter represents any property, however it represents built-in variable in the function equals(watch).
How to resolve the conflict between them?

filterFilter: https://github.com/angular/angular.js/blob/master/src/ng/filter/filter.js#L235
equals(watch): https://github.com/angular/angular.js/blob/master/src/Angular.js#L992
Smart-Table: https://github.com/lorenzofox3/Smart-Table/blob/master/src/stTable.js#L122

@petebacondarwin
Copy link
Contributor

@javeme You could supplement your watch with value of this property directly...

//object to search 
$scope.search = {
  '$': 'something...',
  // ... other filtering properties
};

function filterResults() {
  results = filter(list, search);
} 
// watch the search object (except the `$` property)
$scope.$watch('search', filterResults, true);
// watch the `$` property
$scope.$watch('search.$', filterResults);

This would not work for situations where $ appears further down in the search object.

Perhaps this is another use case for #10096

@LiZhangmei
Copy link

@petebacondarwin Thanks for your solution, if we already know the bug we could bypass it. but it would be better If we can completely solve it.

@petebacondarwin
Copy link
Contributor

Perhaps the easiest solution here would be to allow the "match any property" identifier symbol (currently $) to be configured on a filter by filter basis?

The filterFilter could have an extra parameter:

//object to search 
$scope.search = {
  '%': 'something...',
  // ... other filtering properties
};

function filterResults() {
  results = filter(list, search, undefined, '%');
} 
// watch the search object (including the `%` property)
$scope.$watch('search', filterResults, true);

Now the watch would pick up changes to the % properties. Would that be acceptable?

@gkalpak do you have any thoughts on this?

@gkalpak
Copy link
Member

gkalpak commented Nov 18, 2015

@petebacondarwin, your suggestion sounds reasonable. It should be easy to implement and not introducing any BC, so why not ?

#10096 would offer an equally good solution (but both can co-exist).

I am fine with both.

Admittedly, it wasn't a great choice to use $ (which is considered Angular-proprietary) for something that we expect the user to specify.

If we considered this a bug and wanted to fix it, maybe we could add an extra check, wherever we check (and ignore) properties starting with $, to also check that the property isn't just $ (in which case it shouldn't be ignored).
(I'd be a little BC though.)

So, all things considered, I think @petebacondarwin's suggestion gives the best "value-for-money" 😃

@petebacondarwin
Copy link
Contributor

Admittedly, it wasn't a great choice to have use $ (which is considered Angular-proprietary) for something that we expect the user to specify.

I did think of saying this but thought it would not be fair on the person who wrote the code - especially as I reviewed it :->

PRs welcome!

@gkalpak
Copy link
Member

gkalpak commented Nov 18, 2015

I did think of saying this but thought it would not be fair on the person who wrote the code

Nah...Angular has (almost) always taken pride in having a large community and a great team. There's no person in Angular, it's always a team. I didn't mean to be judgemental in any case 😃

  • especially as I reviewed it :->

Are you sure ? It seems that the special $ filter property has been there since the very very first commit to this repo.

That said, I obviously spoke too soon. It's the other way around: $ as a special filter property was there before standardizing the convention of prefixing Angular-specific properties with $/$$.
(Useless info of the day: ng was nglr back then. Go figure !)

@gkalpak gkalpak self-assigned this Nov 19, 2015
gkalpak added a commit to Athens-AngularJS-Meetup/angular.js that referenced this issue Nov 19, 2015
Previously, the special property name that would match against any
property was hard-coded to `$`.
With this commit, the user can specify an arbitrary property name,
by passing a 4th argument to `filterFilter()`. E.g.:

```js
var items = [{foo: 'bar'}, {baz: 'qux'}];
var expr  =  {'%': 'bar'};

console.log(filterFilter(items, expr, null, '%'));   // [{foo: 'bar'}]
```

Fixes angular#13313
gkalpak added a commit to Athens-AngularJS-Meetup/angular.js that referenced this issue Nov 20, 2015
Previously, the special property name that would match against any
property was hard-coded to `$`.
With this commit, the user can specify an arbitrary property name,
by passing a 4th argument to `filterFilter()`. E.g.:

```js
var items = [{foo: 'bar'}, {baz: 'qux'}];
var expr  =  {'%': 'bar'};

console.log(filterFilter(items, expr, null, '%'));   // [{foo: 'bar'}]
```

Fixes angular#13313
Narretz pushed a commit that referenced this issue Jun 16, 2016
Previously, the special property name that would match against any
property was hard-coded to `$`.
With this commit, the user can specify an arbitrary property name,
by passing a 4th argument to `filterFilter()`. E.g.:

```js
var items = [{foo: 'bar'}, {baz: 'qux'}];
var expr  =  {'%': 'bar'};

console.log(filterFilter(items, expr, null, '%'));   // [{foo: 'bar'}]
```

Fixes #13313
PR (#13356)
Narretz pushed a commit that referenced this issue Jun 16, 2016
Previously, the special property name that would match against any
property was hard-coded to `$`.
With this commit, the user can specify an arbitrary property name,
by passing a 4th argument to `filterFilter()`. E.g.:

```js
var items = [{foo: 'bar'}, {baz: 'qux'}];
var expr  =  {'%': 'bar'};

console.log(filterFilter(items, expr, null, '%'));   // [{foo: 'bar'}]
```

Fixes #13313
PR (#13356)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants