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

docs$rootScope.Scope#$watchGroup: clarify listener's oldValues parameter #12643

Closed
wants to merge 1 commit into from
Closed

docs$rootScope.Scope#$watchGroup: clarify listener's oldValues parameter #12643

wants to merge 1 commit into from

Conversation

rehmsen
Copy link

@rehmsen rehmsen commented Aug 21, 2015

Mention explicitly that oldValues may contain changes that happened before the last call to listener, as this seems non-obvious, see

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@smashercosmo
Copy link

The most valuable PR ever. And I'm only half-joking)) This was a great source of confusion for me.

@Narretz Narretz added this to the Backlog milestone Aug 21, 2015
Narretz
Narretz reviewed Aug 21, 2015
View reviewed changes
src/ng/rootScope.js
@@ -418,8 +418,9 @@ function $RootScopeProvider() {
* expression in `watchExpressions` changes
* The `newValues` array contains the current values of the `watchExpressions`, with the indexes matching
* those of `watchExpression`
* and the `oldValues` array contains the previous values of the `watchExpressions`, with the indexes matching
* those of `watchExpression`
* and the `oldValues` array contains the value of each `watchExpression` before the last change (it
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is clear enough. My head actually hurts a bit reading it. I will also see if I can come up with a better wording.

Copy link
Author

Choose a reason for hiding this comment

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

I agree it is not perfect. It is pretty tricky to describe this behavior concisely and clearly. New attempt:

and the oldValues array contains the value of each watchExpression before it was last changed (even if that change did not happen in this digest cycle).

Happy for any suggestion with regards to wording - as long as we mention this for me unexpected behavior explicitly. If you can tell which part exactly find hard to understand, I will try to improve that specifically.

@Narretz
Copy link
Contributor

Narretz commented Aug 21, 2015

Thanks for the PR, this is important info indeed. I think we should also add an example that shows how the watchGroup executes the listener, and with what values.

@rehmsen
Copy link
Author

rehmsen commented Aug 24, 2015

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@tobyee tobyee mentioned this pull request May 22, 2017
Closed
3 tasks
Narretz added a commit to Narretz/angular.js that referenced this pull request May 22, 2017
@Narretz
docs($rootScope.Scope): clarify $watchGroup "oldValues" argument
e0177a4 
This should help to prevent issues such as angular#8671, angular#12452, angular#16004.

Closes angular#12643
@Narretz Narretz mentioned this pull request May 22, 2017
Merged
3 tasks
Narretz added a commit to Narretz/angular.js that referenced this pull request Jun 29, 2017
@Narretz
docs($rootScope.Scope): clarify $watchGroup "oldValues" argument
2d3069c 
This should help to prevent issues such as angular#8671, angular#12452, angular#16004.

Closes angular#12643
Narretz added a commit to Narretz/angular.js that referenced this pull request Jun 29, 2017
@Narretz
docs($rootScope.Scope): clarify $watchGroup "oldValues" argument
4d20781 
This should help to prevent issues such as angular#8671, angular#12452, angular#16004.

Closes angular#12643
Narretz added a commit that referenced this pull request Jun 29, 2017
@Narretz
docs($rootScope.Scope): clarify $watchGroup "oldValues" argument
06baf18 
This should help to prevent issues such as #8671, #12452, #16004.

Note that the behavior will change in 1.7 (see #15854)

Closes #12643
Closes #16005
@Narretz
Copy link
Contributor

Narretz commented Jun 29, 2017

Docs have been updated in 06baf18

@Narretz Narretz closed this Jun 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes needs: work type: docs
Projects
None yet
Milestone
Backlog - won't fix
Development

Successfully merging this pull request may close these issues.
4 participants
@rehmsen @googlebot @smashercosmo @Narretz