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

One phase initialization for providers #418

Closed
remojansen opened this issue Nov 21, 2016 · 3 comments
Closed

One phase initialization for providers #418

remojansen opened this issue Nov 21, 2016 · 3 comments
Assignees
Milestone

Comments

@remojansen
Copy link
Member

Expected Behavior

Reduce the number of steps required by the users when using synchronous factories (AKA Providers).

Current Behavior

Asynchronous factories require two steps for initialization:

class UserRepository { 

    private _db: DbClient;
    private _dbProvider: Provider<DbClient>;

    // STEP 1
    public constructor(
        @inject("Provider<DbClient>") provider: Provider<DbClient> // Provider injection 
    ) { 
        this._dbProvider = provider;
    }

    // STEP 2
    private async init() {
        if (this._db) return this._db;
        this._db = await this._dbProvider();
        return Promise.resolve(this._db);
    }

    public async getUser(): Promise<Users[]>{
        let db = await this.init();
        return db.collections.user.get({});
    }

    public async deletetUser(id: number): Promise<boolean>{
        let db = await this.init();
        return db.collections.user.delete({ id: id });
    }

}

Possible Solution

As suggested by @buehler:

class Provider<T>{
    public readonly value: T;

    public get someFancyNameForProvideValue(): Promise<T> {
        return this.value !== undefined ? Promise.resolve(this.value): this.provide();
    }

    public provide(): Promise<T> {
         /* exec provider function */
         this.value = 42; // save the result of the provider function
    }
}

// then you could make either:

class UserRepository { 

    // STEP 1
    public constructor(
        @inject("Provider<DbClient>") private provider: Provider<DbClient> // Provider injection 
    ) {}

    public async getUser(): Promise<Users[]>{
        let db = this.provider.value || await this.provider.provide();
        return db.collections.user.get({});
    }
}

// or (would be cool too, with the fancy getter):

class UserRepository { 

    // STEP 1
    public constructor(
        @inject("Provider<DbClient>") private provider: Provider<DbClient> // Provider injection 
    ) {}

    public async getUser(): Promise<Users[]>{
        let db = await this.provider.someFancyNameForProvideValue;
        return db.collections.user.get({});
    }
}

Context

#404

@remojansen remojansen self-assigned this Nov 26, 2016
@remojansen remojansen added this to the 3.0.0 milestone Nov 29, 2016
@remojansen remojansen mentioned this issue Dec 4, 2016
9 tasks
@remojansen
Copy link
Member Author

I started working on this and I realized that the proposed solution has a problem: It assumes that we want the provider to return a singleton. For this reason, I decided not to go ahead with this change. However, I noticed that the Provider was much less flexible than the Factory. I have upgraded the Provider signature to allow advanced use cases like:

  • Singleton and Transient scope support
  • Custom arguments support
  • Support for the partial application of custom arguments

I have created a PR #431 that documents all these features. The PR also documents a "recipe" that helps to reduce boilerplate:

function valueOrDefault<T>(provider: () => Promise<T>, defaultValue: T) {
    return new Promise<T>((resolve, reject) => {
        provider().then((value) => {
            resolve(value);
        }).catch(() => {
            resolve(defaultValue);
        });
    });
}
@injectable()
class Ninja {
    public level: number;
    public rank: string;
    public constructor() {
        this.level = 0;
        this.rank = "Ninja";
    }
    public train(): Promise<number> {
        return new Promise<number>((resolve) => {
            setTimeout(() => {
                this.level += 10;
                resolve(this.level);
            }, 100);
        });
    }
}

@injectable()
class NinjaMaster {
    public rank: string;
    public constructor() {
        this.rank = "NinjaMaster";
    }
}

type NinjaMasterProvider = () => Promise<NinjaMaster>;

let container = new Container();

container.bind<Ninja>("Ninja").to(Ninja).inSingletonScope();
container.bind<NinjaMasterProvider>("NinjaMasterProvider").toProvider((context) => {
    return () => {
        return new Promise<NinjaMaster>((resolve, reject) => {
            let ninja = context.container.get<Ninja>("Ninja");
            ninja.train().then((level) => {
                if (level >= 20) {
                    resolve(new NinjaMaster());
                } else {
                    reject("Not enough training");
                }
            });
        });
    };
});

let ninjaMasterProvider = container.get<NinjaMasterProvider>("NinjaMasterProvider");

valueOrDefault(ninjaMasterProvider, { rank: "DefaultNinjaMaster" }).then((ninjaMaster) => {
    // Using default here because the provider was rejected (the ninja has a level below 20)
    expect(ninjaMaster.rank).to.eql("DefaultNinjaMaster");
});

valueOrDefault(ninjaMasterProvider, { rank: "DefaultNinjaMaster" }).then((ninjaMaster) => {
    // A NinjaMaster was provided because the the ninja has a level above 20
    expect(ninjaMaster.rank).to.eql("NinjaMaster");
    done();
});

remojansen added a commit that referenced this issue Dec 4, 2016
* Remove default exports

* Upgraded tslint

* Implemented #408

* Implementes #419

* fixes #405

* release 3.0.0-beta.2

* Implements #421 & refactor enum -> literal types

* Added #421 docs

* Working on #193

* Working on #193

* Working on #193

* Working on #193

* Working on #193

* Working on #193

* Working on #193

* Working on #193

* [WIP] #418

* [WIP] #418
@gagle gagle mentioned this issue Apr 16, 2017
@bal1anD
Copy link

bal1anD commented Mar 23, 2019

Any updates on this feature?

@parisholley
Copy link
Contributor

@bal1anD check out #1074

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