Match based on function-names (was: Wrapping DDO)

[kentcdodds]

I'm developing a library called angular-scope-types which (currently) requires you to wrap your DDO in a function call, like this:

angular.module('someModule').directive('fooDir', function(scopeTypesDirective) {
  return scopeTypesDirective({
    // ... some config
    controller: FooDirCtrl
    // ... some more config
  });

  function FooDirCtrl(fooService, barFactory) {
    // stuff in here
  }
});

However, ng-annotate doesn't recognize the FooDirCtrl as injectable and I have to use explicit annotation :-(

Is there something I can do to make ng-annotate recognize this pattern?

[Rob--W]

By "explicit annotation", I guess that you've an aversion against "explicit annotation" in the sense that you don't want to type the dependencies twice, right? If so, then you can still get ng-annotate to work with the ngInject explicit annotation (ngInject directive, like 'use strict'; , or comments, see README):

angular.module('someModule').directive('fooDir', function(scopeTypesDirective) {
  return scopeTypesDirective({
    // ... some config
    controller: FooDirCtrl
    // ... some more config
  });

  function FooDirCtrl(fooService, barFactory) {
    'ngInject'; // <------------------------------------- Explicit annotation
    // stuff in here
  }
});

Result:

angular.module('someModule').directive('fooDir', ["scopeTypesDirective", function(scopeTypesDirective) {
  FooDirCtrl.$inject = ["fooService", "barFactory"]; // <---------- FooDirCtrl dependencies
  return scopeTypesDirective({
    // ... some config
    controller: FooDirCtrl
    // ... some more config
  });

  function FooDirCtrl(fooService, barFactory) {
    'ngInject';
    // stuff in here
  }
}]);

[kentcdodds]

The point of this issue is to see if there's a way that ng-annotate can annotate my directive's controller for me without me having to use the 'ngInject' or // @ngInject annotations (which is what I'm doing right now).

[Rob--W]

I think that it's VERY optimistic to assume that ng-annotate can work with your code without an explicit annotation. By static code analysis, it's not possible to know whether the code snippet you've shown requires DI for FooDirCtrl.

On the other hand, it could be possible to check whether a function is made public, and if so, automatically add .$inject to function declarations. This method will have false positives and thus add unnecessary $injects. Perhaps the feature can be added behind a flag?

[kentcdodds]

What would be optimal for me is if I can configure ng-annotate to say that all files who's filename matches some glob should annotate any functions that match another pattern. Something like this:

{
  customAnnotationOptions: [
    {
      test: 'app/**/*.js',
      annotateFnNameMatches: [
        /.*Ctrl$/
      ]
    }
}

Doing something like this would give me a TON of power to have ng-annotate support my custom style.

[olov]

Sorry for the late response to this. @kentcdodds you can do that now already with a very simple plugin:

create plugin-fnname.js:

var ctx;
module.exports = {
    init: function(_ctx) {
        ctx = _ctx;
    },
    match: function(node) {
        var isFn = ctx.isFunctionExpressionWithArgs(node) || ctx.isFunctionDeclarationWithArgs(node);
        if (isFn && node.id && /Ctrl$/.test(node.id.name)) {
            ctx.addModuleContextIndependentSuspect(node, ctx);
        }
    },
};

tests-issues/issue171.js:

angular.module('someModule').directive('fooDir', function(scopeTypesDirective) {
    return scopeTypesDirective({
        // ... some config
        controller: FooDirCtrl
        // ... some more config
    });

    function FooDirCtrl(fooService, barFactory) {
        // stuff in here
    }
    function bar(x) {}
    var bazCtrl = function bazCtrl(a) {}; // need to give function expr an inner name here
});

ng-annotate

ng-annotate --plugin plugin-fnname.js -a tests-issues/issue171.js

output:

angular.module('someModule').directive('fooDir', ["scopeTypesDirective", function(scopeTypesDirective) {
    FooDirCtrl.$inject = ["fooService", "barFactory"];
    return scopeTypesDirective({
        // ... some config
        controller: FooDirCtrl
        // ... some more config
    });

    function FooDirCtrl(fooService, barFactory) {
        // stuff in here
    }
    function bar(x) {}
    var bazCtrl = ["a", function bazCtrl(a) {}]; // need to give function expr an inner name here
}]);

Most ng-annotate tools should let you pass your own plugin to it. If you were using ng-annotate directly as an API, you'd pass the plugin like this:

var fnnameplugin = (function() {
    var ctx;
    return {
        init: function(_ctx) {
            ctx = _ctx;
        },
        match: function(node) {
            var isFn = ctx.isFunctionExpressionWithArgs(node) || ctx.isFunctionDeclarationWithArgs(node);
            if (isFn && node.id && /Ctrl$/.test(node.id.name)) {
                ctx.addModuleContextIndependentSuspect(node, ctx);
            }
        },
    };
})();

var resObj = ngAnnotate(src, {add: true, plugin: [fnnameplugin]});

As for restricting to certain filenames, you'll handle this on the level above ng-annotate (your build tool). ng-annotate as an API does not know of filename.

This plugin is created to match any function with a name that ends with Ctrl, but you can change the code freely - node is just an Esprima-compatible AST node (parsed by Acorn). ctx.isFunction... are just simple helpers, you could write var isFn = (node.type === "FunctionExpression" || node.type === "FunctionDeclaration") && node.params.length >= 1; instead. The most common way of expressing a match in a plugin is by doing return node, however this will still restrict the node to being inside an angular-detected context (i.e. inside angular.module(...)). To relax this and match unconditionally of angular-context we do ctx.addModuleContextIndependentSuspect(node, ctx) instead (and don't have to return anything). So now this plugin will match a top-level function FooCtrl(a) {}.

Let me know how this works for you!

[kentcdodds]

Very cool. I'll give this a closer look a little later. Thanks for that!

[olov]

If there's broader interest in this functionality then I will consider making the example plugin in #171 (comment) a built-in option.

[alexpeattie]

👍 This would be really handy to have built-in ☺️

[olov]

#245