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

fix (filterFilter): #6098

Closed
wants to merge 2 commits into from
Closed

fix (filterFilter): #6098

wants to merge 2 commits into from

Conversation

amj
Copy link
Contributor

@amj amj commented Feb 3, 2014

allow both dotted predicate object fields, and nested predicate objects in filters.

@@ -188,7 +188,7 @@ function filterFilter() {
(function(path) {
if (typeof expression[path] == 'undefined') return;
predicates.push(function(value) {
return search(path == '$' ? value : (value && value[path]), expression[path]);
return search(path == '$' ? value : ( (value && value[path]) || getter(value, path)), expression[path]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this behavior could lead to some pretty interesting collisions.

I think it would be better to use different syntax to distinguish between these two behaviors.

We thought about prefixing path with $ since that is already being used, but it just looks weird:

items | filter: {'$some.deep.path': value}

maybe something like this would be easier to understand:

items | filter: {'path: some.deep.path': value}

but even that looks weird.

another option which is the one that I'm currently preferring is:

items | filter: {some: {deep: {path: value}}}

it's a bit verbose but very explicit - there are no surprises here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the final option is very clear!

Can you elaborate on the collisions? I think the resolution order here is clear:

  1. If path is a dotted property that exists on value, use that.
  2. Otherwise, it traverses an object as expected.
    Can you give an example of the collisions you're expecting?

Considering that dotted property names are usually linter errors, and generally are artifacts of a serializer, i think you can make the case that nested objects are a much more natural usecase than supporting dotted properties. Whatever that means for an intuitive syntax, though, I don't know :) The last one looks a bit like MongoDB's query language, so maybe it's less surprising than I think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
  'some.path': val1,
  some: { path: val2
}

object like this could be pretty confusing to work with if we simply used the fallback method.

I agree that it's not common, but I'd rather avoid it altogether.

@IgorMinar IgorMinar self-assigned this Feb 3, 2014
@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@tbosch tbosch modified the milestones: 1.2.13, 1.2.12 Feb 7, 2014
@IgorMinar
Copy link
Contributor

fixed by #6215

@IgorMinar IgorMinar closed this Feb 13, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants