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

Register single dependency #120

Closed
ceilfors opened this issue Jun 20, 2019 · 6 comments
Closed

Register single dependency #120

ceilfors opened this issue Jun 20, 2019 · 6 comments

Comments

@ceilfors
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
When I need to register a single dependency, the current Laconia API is asking me to return an object, which lead to a verbose code.

const instances = ({ env }) => ({
  orderStream: new KinesisOrderStream(env.ORDER_STREAM_NAME)
});

exports.handler = laconia(kinesis(app))
  .register(instances);

Describe the solution you'd like

exports.handler = laconia(kinesis(app))
  .register("orderStream", ({env}) => new KinesisOrderStream(env.ORDER_STREAM_NAME) );

This will also allow users to decouple their factories better from Laconia:

exports.handler = laconia(kinesis(app))
  .register("orderStream", ({env}) => myOwnFactory(env.ORDER_STREAM_NAME) );
@hugosenari
Copy link
Contributor

hugosenari commented Aug 12, 2019

Suggestion, add helper to convert to expected type:

const simple = (obj = {}) => deps => Object
    .entries(obj)
    .reduce((result, [key, fn]) => ({ ...result, [key]: fn(deps) }), {});

const constants = (obj = {}) => () => Object
    .entries(obj)
    .reduce((result, [key, value]) => ({ ...result, [key]: value }), {});

usage:

laconia(app)
    .register(simple({
        orderStream: ({env}) =>  myOwnFactory(env.ORDER_STREAM_NAME)
    }))
    .register(constants({
        ONE: 1,
        foo: 'bar'
    })); 

@ceilfors
Copy link
Collaborator Author

That's an interesting pattern! I guess if I want to register a single dependency, we can do it like this?

laconia(app)
    .register(single("orderStream", ({env}) =>  myOwnFactory(env.ORDER_STREAM_NAME)))

What are the pro and cons between the two approaches?

@hugosenari
Copy link
Contributor

Adapter Pattern Method Overload
Add feature without change core code OK NOK
Easier to test OK NOK
Less verbose NOK OK
Less imports NOK OK

@ceilfors
Copy link
Collaborator Author

Thanks @hugosenari! It looks like the pro and cons can be derived into:

Adapter Pattern Method Overload
Framework maintainability OK NOK
User experience NOK OK

From the framework point of view, I think we should prefer user experience. On the other hand, I don't think they're mutually exclusive, if there are enough common adapter patterns in the user land, we should try to absorb it into the framework.

That said, would you be interested to make a PR for this feature?

hugosenari pushed a commit to hugosenari/laconia that referenced this issue Sep 10, 2019
@hugosenari
Copy link
Contributor

hugosenari commented Sep 11, 2019

The pull request that add .set(name, factory, options) as alias for .register(factory, options);

I'm not sure about naming, but is better than my other approach that tried to overload register. ;-)

@ceilfors
Copy link
Collaborator Author

@hugosenari I didn't realize how difficult it is to overload things in JavaScript, my bad. Maybe adding a new method is a good way to go, I'll continue the conversation in the PR.

hugosenari pushed a commit to hugosenari/laconia that referenced this issue Sep 11, 2019
…onia/packages/laconia-core/test/types/index.ts`
geoffdutton added a commit to geoffdutton/laconia that referenced this issue Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants