toddmotto / angularjs-styleguide

AngularJS styleguide for teams
https://ultimateangular.com
5.96k stars 700 forks source link

Component Controller - Inject new service require change in 3 place #85

Open phucpnt opened 8 years ago

phucpnt commented 8 years ago

image

Hello,

Whenever you inject a new service, you need to change 3 place: 1) register the server 2) add new params into the constructor 3) assign the service into the component this. Please check the image I attach. Should we simplify those requirements?

d3lm commented 8 years ago

@phucpnt First thing you can do is to remove the assignment of your Service to a variable in constructor. If you declare your Service parameter as private or public can can omit the assignment, because this will happen automatically when you add a modifier to your parameter. At least thats what I do. Another thing is that you could use a task which runs ngAnotate, which I would highly recommend. When using ngAnnotate you can also omit the $inject statemenet. However, you will need to add ngInject into your constructor. The downside to this is that you must use the name of the Service that was used to register the service. So you cannot e.g. write ts (for TodoService). At the end you only have one place where the Service is being typed out. Later on, if you change the name or anything else you could find and replace all occurrences of your service within a file or what ever it is you wanna do then. Personally, I am not 100% sure what is best - using the $inject or a build step and omitting this. Happy to discuss this with other people.

phucpnt commented 8 years ago

@d3lm thanks for your comments. I'm still thinking about how to make the connection between the different services more easily to grasp by. I has a few idea regards the dependency injection but those not connected well and give me clearer picture till now... I will keep you posted.

floriangosse commented 8 years ago

@phucpnt I wrote a small function which automatically bind the injected dependencies to this. And if you use ngAnnotate you only need to define your dependencies as parameters of your constructor.

Check out the class wrapper:

function binder(constructor) {
   function wrapConstructor () {
      var args = Array.prototype.slice.call(arguments);

      constructor.$inject.reduce((self, depName, depIndex) => {
         self[depName] = args[depIndex];

         return self;
      }, this);

      return constructor.apply(this, args);
   }
   wrapConstructor.prototype = constructor.prototype;
   wrapConstructor.$inject = constructor.$inject;
   return wrapConstructor;
}

// Usage:
//    module.exports = binder(YourController);

Note: I didn't test it with angular. I adapted the instantiation of angular for testing.

AkosLukacs commented 8 years ago

@floriangosse Looks good, as long as you don't minify your code. But I guess ngAnnote can solve that. A sidenote: drawback of class decorators (those could be used to wire up the controller and dependency injection) is that the class name, and constructor parameters are not emitted as metadata. Therefore lost at minification, if you mangle the parameter and class names during minification.

caioiglesias commented 8 years ago

If you declare your Service parameter as private or public can can omit the assignment

@d3lm how exactly? Typescript?

floriangosse commented 8 years ago

@AkosLukacs Sure, if you minify your code without ngAnnotate it will break but application. But you should use strictDi and ngAnnotate.

d3lm commented 8 years ago

@caioiglesias Yes, I was refering to TypeScript. The constructor would look like this:

constructor(private FooService) { }

Then FooService is then available by accessing this.FooService in your class anywhere you want.