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

feat(directive): support injecting required controllers into a directive controller #6040

Closed
wants to merge 2 commits into from
Closed

feat(directive): support injecting required controllers into a directive controller #6040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e34f00
fix(directive): issue 5893, injecting required controllers into contr…
mk-job Jan 29, 2014
5d3f947
feat(directive): issue 5893, implementation has created exceptions
mk-job Jan 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions docs/content/guide/directive.ngdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,10 @@ option. Just like `ngController`, this option attaches a controller to the templ

Looking back at `myPane`'s definition, notice the last argument in its `link` function: `tabsCtrl`.
When a directive requires a controller, it receives that controller as the fourth argument of its
`link` function. Taking advantage of this, `myPane` can call the `addPane` function of `myTabs`.
`link` function or will be injected in your controller as $requiredControllers. Taking advantage of this, `myPane` can call the `addPane` function of `myTabs`.

Savvy readers may be wondering what the difference is between `link` and `controller`.
The basic difference is that `controller` can expose an API, and `link` functions can interact with
controllers using `require`.
The basic difference is that `controller` can expose an API, and `link` functions not. They both can interact with controllers using `require`.

<div class="alert alert-success">
**Best Practice:** use `controller` when you want to expose an API to other directives.
Expand Down
20 changes: 19 additions & 1 deletion src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1477,11 +1477,29 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
transcludeFn = boundTranscludeFn && controllersBoundTransclude;
if (controllerDirectives) {
forEach(controllerDirectives, function(directive) {
// github issue #5893 Add ability to inject required controllers
// into the controller function
var requiredControllers;
if (directive.require && directive.require !== directive.name) {
// the directive requires its own controller. this would make an error
// this does not prevent require: "^sameDirective"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would this make an error? It's perfectly legal for a directive to require its own controller

... Oh, I guess you mean like a circular dependency issue when injecting these into the controller itself. I see.

Copy link
Author

Choose a reason for hiding this comment

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

@caitp at this point you want to instantiate the controller and need to resolve the injection locals here. Before it is instantiated, it is not in the data-property of the dom-element. This means it cannot be required / injected in this way. I don't see any reason why a controller should require itself (keyword: this!). I see valid ways there the controller requries a parent-controller of the same kind like it is by the form directives, but this does not prevent that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you certainly can get the controller of sibling directives from the expandostore in the controller closure! Unless you're using certain versions of jQuery which are currently slightly broken (in combination with angular, such as v2.0.x), this does work and I have apps showcasing it.

But at any rate, this is a feature and it's going to be some time before it gets merged, if it gets merged at all. There are already tools available to do this stuff.

var requiredControllersCopy = angular.copy(directive.require);
if (angular.isArray(requiredControllersCopy)) {
var indexOfSelf = requiredControllersCopy.indexOf(directive.name);
if (indexOfSelf !== -1) {
requiredControllersCopy.splice(indexOfSelf, 1);
}
}
requiredControllers = requiredControllersCopy
&& getControllers(requiredControllersCopy, $element, elementControllers);
}

var locals = {
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
$element: $element,
$attrs: attrs,
$transclude: transcludeFn
$transclude: transcludeFn,
$requiredControllers: requiredControllers
}, controllerInstance;

controller = directive.controller;
Expand Down