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

refactor($parse): change 'this' to a $parse keyword instead of scope field #9105

Closed
wants to merge 1 commit into from

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented Sep 16, 2014

BREAKING CHANGE:

  • $scope['this'] no longer exits on the $scope object
  • $parse-ed expressions no longer allow chaining 'this' such as this['this'] or $parent['this'] or this.this.this.this
  • 'this' in $parse-ed expressions can no longer be overridden, if a variable named 'this' is put on the scope it must be accessed using this['this']

I think this is a cleaner way of implementing 'this' which is more like other keywords such as null or false. But it is also a breaking change for anyone that did weird things such as this.this.this.this (and less weird: $scope['this'] = ...).

@caitp
Copy link
Contributor

caitp commented Sep 16, 2014

This is good to me.

@caitp
Copy link
Contributor

caitp commented Sep 18, 2014

I would actually be interested in seeing the performance difference between this and properties, though. One potential issue with it is that it overwrites the this property (if any) in whatever parse context is used, although I can't imagine why people would put this on their parse contexts.

It's kind of an elegant solution though

@@ -128,7 +128,7 @@ function $RootScopeProvider(){
this.$$phase = this.$parent = this.$$watchers =
this.$$nextSibling = this.$$prevSibling =
this.$$childHead = this.$$childTail = null;
this['this'] = this.$root = this;
this.$root = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

since you're changing this, please remove the extra space character after the =

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@caitp caitp added this to the 1.3.0 milestone Sep 19, 2014
@caitp caitp self-assigned this Sep 19, 2014
@jbedard
Copy link
Contributor Author

jbedard commented Sep 19, 2014

That potential issue is one of the breaking changes. I think it is a good thing though, now it is consistent with accessing other keyword properties such as 'null', 'false', etc.

The performance when using 'this' will be worse because it will now be a field-access (which wraps 2 getters), where previously it was a single getter. But I think the use of 'this' in expressions is rare enough that removing this['this'] = this from every scope instance is well worth it.

@caitp
Copy link
Contributor

caitp commented Sep 22, 2014

@tbosch can you lgtm this?

There's a tiny breaking change, but I don't think it's likely to affect anyone really

…field

BREAKING CHANGE:
- $scope['this'] no longer exits on the $scope object
- $parse-ed expressions no longer allow chaining 'this' such as this['this'] or $parent['this']
- 'this' in $parse-ed expressions can no longer be overriden, if a variable named 'this' is put on the scope it must be accessed using this['this']
@IgorMinar
Copy link
Contributor

lgtm

@IgorMinar IgorMinar closed this in 5572b40 Sep 27, 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.

5 participants