Skip to content

Directive definition in a variable #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 20, 2018

Conversation

noppa
Copy link

@noppa noppa commented Nov 20, 2017

Adds implicit annotation support for directive definitions
that are returned from a variable.

angular.module('MyMod').directive('foo', function() {
  var directive = {
    // Ctrl wasn't annotated before, but will be now.
    controller: function Ctrl(FooService) {},
  };
  return directive;
});

Currently, implicit annotations only work for a directive if the
directive definition is returned as an object literal.

angular.module('MyMod').directive('foo', function() {
  return {
    // Ctrl is correctly annotated.
    controller: function Ctrl(FooService) {},
  };
});

We have used the former quite a lot in our codebase, so we
needed to add support for it. I understand that the general
recommendation for more complex use cases like this is to add
explicit annotations, but here's a PR for the feature anyway
if you are interested. All the tests pass and I added one more
to test this specific case.

Adds implicit annotation support directive definitions that are returned
from a variable.

```javascript
angular.module('MyMod').directive('foo', function() {
  var directive = {
    // Ctrl wasn't annotated before, but will be now.
    controller: function Ctrl(FooService) {},
  };
  return directive;
 });
```
@schmod
Copy link
Owner

schmod commented Nov 20, 2017

Looks good to me! Thanks for the contribution!

@noppa
Copy link
Author

noppa commented Nov 20, 2017

No problem, thanks for the awesome plugin!

@schmod schmod merged commit b5398d4 into schmod:master May 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants