pbastowski / angular2-now

Angular 2 @Component syntax for Angular 1 apps
MIT License
145 stars 15 forks source link

Service providers and factories #20

Closed eXaminator closed 9 years ago

eXaminator commented 9 years ago

Hi, would it be possible to add annotations like @ServiceFactory and @ServiceProvider?

I understand that these are concepts that (afaik) won't find their way into angular 2, but as @Service and @Filter currently are pretty much angular 1.x only, I thought those annotations might also help to keep everything in line.

Great work by the way, I really like this :-)

pbastowski commented 9 years ago

@eXaminator Sorry for the late reply.

Sure, it is possible and they would be useful at times. Are you able to code the JS functions that implement @ServiceFactory and @ServiceProvider by looking at how I implemented @service - see below? Unfortunately, I don't have the time at this moment to write and test them myself.

   function Service(options) {
        options = options || {};
        // Allow shorthand notation of just passing the name as a string
        if (typeof options === 'string')
            options = { name: options };

        return function (target) {
            angular.module(currentModule)
                .service(nameSpace(options.name), target);

            return target;
        };
    }
eXaminator commented 9 years ago

I'm wondering: Do you have any thought about how to handle injection for the $get method of a provider? Is there any way we can use the annotations on class methods?

pbastowski commented 9 years ago

Yes, we could. However, I think it is easier to do something like this (ES6 code):

var app = angular.module("app", []);

@Provider('cls')
class Cls {

  config () {
    console.log('provider config');
  }

  $get () {
    return {
      one,
      two
    };

    function one () {
      console.log('function 1');
    }
    function two () {
      console.log('function 2');
    }
  }

}

function Provider(name) {
  return function(target) {

    app.provider(name, target);

    return target
  }
}

app.config(function(clsProvider) {
  clsProvider.config(1, 2, 3);
});

app.run (function(cls) {
  cls.one();
  cls.two();
});

What do you think?

eXaminator commented 9 years ago

Ok, I thought about this a bit more and to be honest: I'm not sure I'm keen on the idea of providing these annotations/decorators anymore. Yes, I know I was the one asking for them in the first place, but hear me out.

  1. It seems that best practice (especially in ES6) is to use angular.service instead of .factory. There is not much you can do in factory that you couldn't do with a simple service/class. And as soon as we start using "classes" as factories to simply create another class (probably by returning some instance of another class in the constructor, similar how filters work now) we get to a really ugly place. So we might just as well ignore factories. See this for some insight to the service vs factory debate: http://blog.thoughtram.io/angular/2015/07/07/service-vs-factory-once-and-for-all.html
  2. I don't like providers anymore. I don't think there will be a replacement in Angular2. And I don't like the "config phase" because if you only allow configuration during that phase it makes it very hard to change the config due to application state changes. But to be honest: I'm not sure if we might really need this anyway, at least this can be properly represented by classes.

I case you'd still like a @Provider decorator, here would be my usage take on this as I think there is stuff missing in your example.

var app = angular.module("app", []);

@Provider('cls')
@Inject(['someProvider'])
class Cls {
  constructor (someProvider) {
    someProvider.doStuff();
  }
  config (val) {
    this.configVal = val;
  }

  @Inject(['$q'])
  $get ($q) {
    return new OtherCls($q.when(this.configVal));
  }
}

Do you see what I mean? In Angular providers have injectable constructors (you can inject providers here) and the $get method is also injectable (with normal services). I think we would need to handle this, but currently the @Injectdecorator can't handle this.

I actually created my own version for @Inject because it was missing some features I would have liked (this one included). I might create a PR for that later, but it would sadly introduce breaking changes because I didn't use an array as parameter but instead simply add each dependency as a separate parameter. Maybe I could add a fallback or something.

pbastowski commented 9 years ago

I agree. I only ever use @Service in my own apps and I don't like providers or config phases either. My code above was just for quick illustration purposes, since you had asked the question :)

BTW: If you have a look at @State you'll see how I hid the inner workings of the config phase.

Could you post your implementation of @Inject?

eXaminator commented 9 years ago

Ok, here is my implementation. Just some notes:

Here is the code:

export function Inject(...deps) {
    return function (target, name, descriptor) {
        let injectable = target;
        if (descriptor) {
            injectable = descriptor.value;
        }

        let existingInjects = injectable.$inject;

        injectable.$inject = [];

        angular.forEach(deps, function (dep) {
            if (injectable.$inject.indexOf(dep) === -1) {
                injectable.$inject.push(dep);
            }
        });

        if (existingInjects) {
            injectable.$inject = injectable.$inject.concat(existingInjects);
        }

        return descriptor || target;
    };
}

Here is an usage example:

@Inject('$scope')
class BaseCls {
  constructor ($scope) {}
}

@Inject('$q', '$element')
class Cls extends BaseCls {
  constructor ($q, $element, ...parentDeps) {
    super(...parentDeps);
  }
}
pbastowski commented 9 years ago

I like it and will integrate it into angular2-now, including the fallback for deps passed as an array.

Thanks.

eXaminator commented 9 years ago

I forgot to mention: This should also work on all injectable methods. Example:

class ClsProvider {
  @Inject('$q')
  $get($q) {}
}
pbastowski commented 9 years ago

Yes, I saw that :)

pbastowski commented 9 years ago

I have incorporated your Inject functionality in the 3.6 release. Thanks again.

Closing this issue.

eXaminator commented 9 years ago

You are welcome! :)