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

fix(compile): assign controller return value to correct controller #12036

Closed
wants to merge 1 commit into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Jun 5, 2015

Fixes #12029

if (scopeDirective && elementControllers[scopeDirective.name]) {
bindings = scopeDirective.$$bindings.bindToController;
controller = elementControllers[scopeDirective.name];
controller = scopeController = elementControllers[scopeDirective.name];
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 don't know what else scopeDirective can be, so scopeController might not be the best name.

@Narretz Narretz force-pushed the fix-controller-return branch from b28c662 to d4acbf0 Compare June 12, 2015 18:38
@Narretz
Copy link
Contributor Author

Narretz commented Jun 12, 2015

@caitp Can you check this again? The problem is actually much more general. Basically, the return values for each controller was always updated for the same directive.

controller.instance = controllerResult;
$element.data('$' + directive.name + 'Controller', controllerResult);
$element.data('$' + i + 'Controller', controllerResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me, maybe the comment could be a bit more clear --- we're overwriting stuff that was done in setupControllers() because the return value was different from the pre-allocated instance.

@caitp
Copy link
Contributor

caitp commented Jun 12, 2015

lgtm

@Narretz Narretz force-pushed the fix-controller-return branch from d4acbf0 to 98a1717 Compare June 12, 2015 19:24
@Narretz Narretz closed this in 8caf180 Jun 12, 2015
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 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

3 participants