ternjs / tern

A JavaScript code analyzer for deep, cross-editor language support
https://ternjs.net/
MIT License
4.25k stars 378 forks source link

AngularJS plugin doesn't work with vanilla JS function #257

Open angelozerr opened 10 years ago

angelozerr commented 10 years ago

AngularJS plugin doesn't works with vanilla JS function :


function SomeCtrl($scope, $http) { $http. // <-- no completions here

}

Do you think it's possible to manage this commons case?

marijnh commented 10 years ago

How does angular work in this case? How does it inject those dependencies?

magnars commented 10 years ago

It runs toString on the function, looks at the names of the parameters, and then looks for a provider to create an instance to inject. It's a magical mess.

marijnh commented 10 years ago

So it runs through the global scope looking for functions whose name ends in Ctrl, or does it only do it once you use such a function explicitly as a controller? (In the second case, I'm not sure how Tern is supposed to notice that.)

magnars commented 10 years ago

No, you register it as a controller with angular.module('myapp').controller(name, theFn) - and similarly for other angular concepts, like factories, directives, providers and so on.

angelozerr commented 10 years ago

You can see the dodo sample at http://angularjs.org/ :

HTML :


JS :


function TodoCtrl($scope)

I'm not a not expert with AngularJS, but I think $scope is hard coded for AngularJS (I tell that, because if you write function TodoCtrl(MyScope), Angular crashes).

marijnh commented 10 years ago

$scope is not really hard-coded. There's an explicit set of injectables being kept by angular, and it will inject them when asked to. It just needs to know when, and if there is simply a top-level function sitting around with a $scope argument, that's not really enough for the plugin to act on. (Controllers registered with .controller do get their dependencies defined properly.)

angelozerr commented 10 years ago

function TodoCtrl($scope) works with HTML directive ngController. It means that angular parse the HTML :

div align="center" ng-controller="TodoCtrl"

and it search in the Window scope, if they are function named with TodoCtrl, in this case it creates an instance of TodoCtrl and inject the $scope, the $http, etc

So I think to manage this case, Tern AngularJS plugin should check if a global function (ex : TodoCtrl) has $scope parameter, and in this case, the function should be considered as controller (like .controller).

To do that angular.js plugin could check the existing of $scope parameter for each function in the postParse function and call after applyWithInjection.

What do you think about this idea?

marijnh commented 10 years ago

I believe it is also possible to write controller functions without a $scope parameter (the parameter names are just regular dependency injections), so I don't think that the parameter name is a very good heuristic for determining what to treat as a controller.

angelozerr commented 10 years ago

yes you are right, you can write a controller without $scope parameter. Perhaps we could consider that function which have a parameter which starts with $ is a controller? I know it's not perfect, but I find it's really shame that angular.js plugin doesn't manage the case of global function because there is a lot of sample which works without module.

subhaze commented 10 years ago

From Angular's Developer Guide:

"NOTE Although Angular allows you to create Controller functions in the global scope, this is not recommended. In a real application you should use the .controller method of your Angular Module for your application as follows:"

var myApp = angular.module('myApp',[]);

myApp.controller('GreetingCtrl', ['$scope', function($scope) {
    $scope.greeting = 'Hola!';
}]);

http://docs.angularjs.org/guide/controller

Perhaps we could consider that function which have a parameter which starts with $ is a controller

That approach could still, easily, lead to false positives. It would also be unable to correctly identify controllers that have custom services that are injected since it is recommended to not prefix custom variable names with the $ or $$

"Angular Namespaces $ and $$ To prevent accidental name collisions with your code, Angular prefixes names of public objects with $ and names of private objects with $$. Please do not use the $ or $$ prefix in your code."

http://docs.angularjs.org/api

While I was writing an unofficial Angular plugin for Tern I opted out on trying to determine any kind of globals like that since it's not the 'Angular' way of doing things and adds unneeded complexities...

bfricka commented 10 years ago

:+1: @subhaze

The shorthand syntax for creating controllers is for quick mocks and demos. I think it was probably added to make the barrier of entry to learning Angular and DI easier, but there are no scenarios where you would actually want to code this way for an actual Angular project.

I think this is a non-issue.