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

fix($compile): respect return value from controller constructor #10502

Closed
wants to merge 2 commits into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Dec 17, 2014

The return value of the controller constructor is now respected in all cases.

If controllerAs is used, the controller will be re-bound to scope. If bindToController is used, the previous binding $watches (if any) will be unwatched, and bindings re-installed on the new controller.

Blocked on #10467 --- This potentially results in leaks and performance regressions, good review is needed here.

…new/isolate scopes

bindToController is now able to be specified as a convenient object notation:

```
bindToController: {
  text: '@text',
  obj: '=obj',
  expr: '&expr'
},
scope: {}
```

It can also be used in conjunction with new scopes, rather than exclusively isolate scopes:

```
bindToController: {
  text: '@text',
  obj: '=obj',
  expr: '&expr'
},
scope: true
```

Closes angular#10420
@googlebot
Copy link

CLAs look good, thanks!

The return value of the controller constructor is now respected in all cases.

If controllerAs is used, the controller will be re-bound to scope. If bindToController is used,
the previous binding $watches (if any) will be unwatched, and bindings re-installed on the new
controller.
// If result changed, re-assign controllerAs value to scope.
addIdentifier(locals, identifier, instance, constructor || expression.name);
}
}
return instance;
}, {
instance: instance,
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't we need this instance to also be updated, if result !== instance ?
(Could this by why you defined instantiate as a variable, but then forgot to update it's instance property ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need it to not be updated so the controller knows to update bindings. But it is true that instantiate isn't needed anymore

@petebacondarwin petebacondarwin modified the milestones: 1.4.0-rc.2, 1.4.x - jaracimrman-existence Apr 29, 2015
@petebacondarwin petebacondarwin assigned caitp and unassigned gkalpak Apr 29, 2015
@petebacondarwin
Copy link
Contributor

@caitp and @gkalpak is this ready to go?

@caitp
Copy link
Contributor Author

caitp commented May 4, 2015

Version of this landed in 62d514b already

@caitp caitp closed this May 4, 2015
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