Skip to content
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

Cannot use arrow function in declaration #103

Closed
jimhigson opened this issue Jan 21, 2018 · 7 comments
Closed

Cannot use arrow function in declaration #103

jimhigson opened this issue Jan 21, 2018 · 7 comments

Comments

@jimhigson
Copy link

Under any environment that isn't transpiling arrow functions down to normal functions, this will fail:

bottle.service('foo', () => ({ doSomething: () => {} }) )
bottle.container.foo

The reason is because Bottle creates services using 'new' -

var service = function service(name, Service) {
        var deps = arguments.length > 2 ? slice.call(arguments, 2) : null;
        var bottle = this;
        return factory.call(this, name, function GenericFactory() {
            var ServiceCopy = Service;
            if (deps) {
                var args = deps.map(getNestedService, bottle.container);
                args.unshift(Service);
                ServiceCopy = Service.bind.apply(Service, args);
            }
            return new ServiceCopy();
        });
    };
  • and arrow functions cannot be used as constructors. This seems a bit restrictive to me since a lot of the time I don't use the 'this' when I create a new instance of something anyway - I just return an object at the end of the function.

One solution might be to detect functions without prototypes (ie, arrow functions) -

if( !ServiceCopy.hasOwnProperty("prototype") )
   return ServiceCopy();
else 
   return new ServiceCopy();

Alternatively, updating the docs to explicitly state that traditional js functions are required would help a lot. Or maybe throwing an exception when the service is registered. It took a little digging to work out what was wrong here.

@ragboyjr
Copy link
Contributor

Another use case for this would be if you want to use services as funcs instead of classes.

function myService() {
  return function() { /* do something */ }
}
// vs
class MyService {
    do() {/* do something */ }
}

I created a wrapper function myself to implement it:

function fnService(container, name, serviceFactory, ...deps) {
    container.factory(name, (c) => {
        return serviceFactory(...deps.map(dep => c[dep]))
    });
}

function myService(arg1) {
  return () => {
    // do something
  }
} 

fnService(container, 'myService', myService, 'someDep');

@young-steveo
Copy link
Owner

young-steveo commented Jan 24, 2018

@jimhigson The documentation in the readme for bottle.service explicitly mentions the constructor intent:

service(name, Constructor [, dependency [, ...]])
Used to register a service constructor...
Constructor | Function | A constructor function that will be instantiated as a singleton.

Take note of the the parenthesis around your function body here:

bottle.service('foo', () => ({ doSomething: () => {} }) )

The parenthesis around the curly braces forces the code to be a one-line arrow function that returns an object literal. It is equivalent to writing it this way:

bottle.service('foo', () => {
   return { doSomething: () => {} };
});

So essentially you are passing in a function that constructs an object and returns it. This is what the bottle.factory method is for. This should work for you:

bottle.factory('foo', () => ({ doSomething: () => {} }) )

Hope that helps!

@jimhigson
Copy link
Author

Ok, so the practical difference between a service and a factory is that a service is always created by a constructor, whereas a factory is created by a function?

I understand that now, but since constructors and functions in Javascript are for the most part the same thing, I wonder if it would be better to unify them?

@ragboyjr
Copy link
Contributor

@young-steveo Correct me if I'm wrong, but bottle.factory only argument is the container itself. I think @jimhigson and my cases, we were hoping that our "factory" functions would injected with the deps.

@young-steveo
Copy link
Owner

@jimhigson That practical difference is correct.

My thoughts on unifying them:

Under the hood bottle only has one mechanism for creating and returning services; it's the service provider. A provider is a Constructor that exposes a single factory method, $get. This method creates and returns your service. The bottle.factory and bottle.service methods are syntactic sugar layered on top of bottle.provider (because creating providers for every service in userland would be tedious).

When you call bottle.factory, bottle is creating a new generic provider for you on the fly and assigning it's $get method to be your factory function. Then it hands it off to bottle.provider to register your service on the container. reference

When you call bottle.service, bottle is wrapping your Constructor in a generic factory function that looks up the dependencies and injects them, and it hands this function off to bottle.factory. reference (ignore the .bind.apply stuff, that's just a fancy way to pass the arguments for the constructor in older JS environments that lack the Spread syntax).

So, no matter what function you use, bottle registers providers for you.

Now, I could start allowing each method to accept a bag of different things (functions with a prototype, functions without, object literals, etc.) and then attempt to figure out how you want to create the eventual provider, but using specific methods allows you to tell me your intent. If you have something that returns a service, give it to me as a factory. If you have a construct-able service, give it to me as a service. If you want to make a provider yourself, give it to me as a provider. This keeps my code clean (no branching conditions) and there's no magic for the user to understand.

@ragboyjr You are correct, the factory will receive the container. I can consider adding a new method to bottle that will take a factory that is injected with dependencies rather than getting the container, but I would have to be convinced that there is enough demand to justify it.

@ragboyjr
Copy link
Contributor

@young-steveo totally understand, thank you for your time and work on this project.

I'm willing to submit a PR for this feature as well, and if you can merge or close when you feel like a consensus has been reached.

I'm sure i'm in the minority with using functional services vs objects, so I doubt this will be the next hot killer feature in bottleJS, but this would be quite convenient and encourage more functional(ish) style programming.

@young-steveo
Copy link
Owner

@jimhigson I think the factory method should be able to solve your issue. Now, with the release of v1.7.0 you can also use the serviceFactory method.

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

No branches or pull requests

3 participants