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

Lazy Loading (toAsyncValue) support #1074

Closed
wants to merge 18 commits into from

Conversation

parisholley
Copy link
Contributor

@parisholley parisholley commented Mar 27, 2019

Allow for users to opt-in to lazy loaded dependencies by using an asynchronous API.

Description

New container methods for pulling objects out of the container in an asynchronous fashion while respecting container semantics and error if trying to mix synchronous/asynchronous dependencies.

Implementation

container.bind<Type>('name').toAsyncValue(async () => {
  return await myAsyncFunction();
});
container.bind<string>('resolve').toAsyncValue(async () => Promise.resolve('foobar'));

const value = await container.getAsync<Type>('name'); // result of myAsyncFunction
const value = await container.get<Type>('name'); // throws an exception
const value = await container.getAsync<string>('resolve'); // foobar

For anyone wanting to play around with this, you can use the NPM release:

@parisholley/inversify-async:1.0.20

Related Issue

#418
#475
#887
#404

Motivation and Context

Use Case 1: Unit/System Testing
While you technically could create separate containers (with a parent container for common objects) for a unit test environment and system test (database, etc), it adds more code complexity and also requires each system integration be live and working in order to bootstrap the container (this matters, because for instance in the IDE, if I want to run a single test, with a single I/O dependency, I don't have to initiate every connection).

Use Case 2: Common Container Use
Let's say you have a "service" code base that is re-used between a web application, command line interface and batch processing. In the case of the CLI, you have different commands that serve different purpose, eg:

  • Clone production data
  • Invoke third party service

By making I/O dependencies lazy loaded (async), our CLI can wait until runtime to connect to the system that it needs for the task (otherwise, same problem as above, you are forced to connect-to, and have access-to systems that are unrelated to the command at hand).

How Has This Been Tested?

Full suite of tests have been implemented to.

Types of changes

  • Updated docs / Refactor code / Added a tests case (non-breaking change)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the changelog.

…c/async, prevent onActivation from firing until lazy value is available
…e than once (even though it is a singleton). removed support for onActivation and context access in order to keep internal code simple. in theory, the async call can happen at any time (or in-side multiple contexts), so conceptually we should not support access the current context
@parisholley
Copy link
Contributor Author

@remojansen @dcavanagh @Jameskmonger thoughts on this?


};

function convertBindingToInstance(requestScope: interfaces.RequestScope, request: interfaces.Request) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use arrow functions rather than function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reasoning? the prior functions in this file follow this format

src/resolution/resolver.ts Outdated Show resolved Hide resolved
@seizir
Copy link

seizir commented Aug 6, 2019

Would it be possible to add the context to the async function like in toDynamicValue, toFactory, and toProvider? That would make it a lot more useful.

container.bind<string>('text').toAsyncValue(async (context) => {
    const first = await context.container.getAsync<string>('firsthalf');
    const second = context.container.get<string>('secondhalf');
    return first + second;
}

@parisholley
Copy link
Contributor Author

@seizir i wanted to, but it makes the implementation very complex as the async functions can be evaluated at any time and context can change. if you need access to the container, i recommend accessing it via closure instead:

const container = ....

container.bind().toAsyncValue(async () => {
const first = container.get();
})

@seizir
Copy link

seizir commented Aug 7, 2019

@parisholley That solution should work in most cases but not if modules are used because there is no way to access the container from there except via the context.

@seizir
Copy link

seizir commented Aug 7, 2019

toAsyncValue ignores defaultScope

const container = new Container({defaultScope: 'Singleton'});
container.bind('a').toAsyncValue( async () => Math.random());

async function f() {
    console.log(await container.getAsync('a')); // 0.5358571181438059
    console.log(await container.getAsync('a')); // 0.4785245037836447
}

f();
const container = new Container();
container.bind('a').toAsyncValue( async () => Math.random()).inSingletonScope();

async function f() {
    console.log(await container.getAsync('a')); // 0.8363558846768307
    console.log(await container.getAsync('a')); // 0.8363558846768307
}

f();

@parisholley
Copy link
Contributor Author

@seizir i can't reproduce this in a mocha test, what version of my NPM module are you using?

@seizir
Copy link

seizir commented Aug 7, 2019

@parisholley I am using 1.0.12
I created a repository demonstrating the bug https://github.com/seizir/inversify-async-bug

@parisholley
Copy link
Contributor Author

@seizir fixed in 1.0.13

@parisholley
Copy link
Contributor Author

closing and continuing with #1132

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

Successfully merging this pull request may close these issues.

3 participants