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

feat($rootScope): optionally limit depth of $watch recursion #4660

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Oct 26, 2013

New parameter to angular.equals limits depth of recursion (default is infinite recursion).

$watch allows a new parameter to limit the depth of equals() when objectEquality is true. Only effects simple objects or array-like objects.

While creating many $watches is not a totally performant feature, this is useful in the case of many nested directives which look at nested properties of the same object, and only really need to care about a single "level" of nested properties, or perhaps several levels.

New parameter to `angular.equals` limits depth of recursion (default
is infinite recursion).

$watch allows a new parameter to limit the depth of equals() when
objectEquality is true. Only effects simple objects or array-like
objects.
@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@caitp
Copy link
Contributor Author

caitp commented Oct 26, 2013

CLA has been signed previously as Caitlin Potter yadda

@timraymond
Copy link
Contributor

Seems reasonable, I'm curious what your use case for this is @caitp, if you don't mind sharing. I can't come up with an instance where I wouldn't want a watch to fire for a deeply nested attribute.

Part of me would like to see the name of equals() become something more revealing of the new option... fuzzy_equals() comes to mind, but maybe this is just unnecessary bike shedding on my part.

Other than that, gets my 👍

@timraymond
Copy link
Contributor

Update: found your issue, linking here for others who are looking for more context. #4591

@caitp
Copy link
Contributor Author

caitp commented Oct 31, 2013

I'll cover it quickly here so there's no reason to link around:

In a lot of cases it can be simpler (if slightly less performant) to instantiate multiple watches of recursing tree-like structures of data (think of Reddit comments, for instance). It is true that there are solutions to this which would add only a single watch, but it is difficult to engineer and is a bit finnicky.

An example use case can be seen in my Comments directives, where it's dealing with arbitrarily nested data. Each comment adds a $watch (which is not really ideal, but is simpler than the alternative) --- However currently this means that each one is watching their entire subtree, when they only really need to be concerned with their own immediate children


It may not be ideal to have lots of $watches performing smaller tasks vs a single $watch performing a very large task, I'd need to measure this --- But I think using a single watch would also be problematic in a lot of ways as well, especially if you wanted to make use of strategies like Scalyr's disable-watches-for-certain-nodes thing.

So to me, it just makes sense to be able to limit it like this

@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your 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 me 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.

@IgorMinar
Copy link
Contributor

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

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

Successfully merging this pull request may close these issues.

6 participants