inversify / InversifyJS

A powerful and lightweight inversion of control container for JavaScript & Node.js apps powered by TypeScript.
http://inversify.io/
MIT License
11.34k stars 719 forks source link

Create an instance without relying on interfaces [toValue binding] #70

Closed kuncevic closed 8 years ago

kuncevic commented 8 years ago

If I have simple class which is not implementing any interface.

export class Something{
        constructor(*/../*) { }
        public static CustomMethodFromNotReallySomething(*/params/*) { }
    }

And a module which exports NotReallySomething relying on third party node module

export var NotReallySomething= GetNotReallySomethingFromThirdParty();

In my app I am swapping Something with NotReallySomething manually using imports so this way is working for me but I am thinking to improve that so just wondering is there any way I can configure inversify to create a single instantiate of Something or NotReallySomething so I can use it in my calling module like var something = kernel.instance('SomethingOrNotReallySomething'); by configuring inversify.config.ts something like:

if (app.get('env') === 'dev') {
kernel.bind(new TypeInstance<Something>('SomethingOrNotReallySomething'));
} 
else if (app.get('env') === 'stg') {
kernel.bind(new TypeInstance<NotReallySomething>('SomethingOrNotReallySomething'));
}
remojansen commented 8 years ago

With the features available today you could do:

if (app.get('env') === 'dev') {
    kernel.bind(new TypeInstance<ISomething>('ISomething', Something));
} 
else if (app.get('env') === 'stg') {
     kernel.bind(new TypeInstance<ISomething>('ISomething', NotReallySomething));
}

Your types Something and NotReallySomething must implement the same ISomething interface.That ensures that it is safe to replace one class with another as the Liskov substitution principle (from the SOLID principles) states:

“objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.”

It is not possible to do:

kernel.bind(new TypeInstance<Something>('SomethingOrNotReallySomething'));

Because that is compiled into the following:

kernel.bind(new TypeInstance('SomethingOrNotReallySomething'));

So as you can see we would not have the type (Something) information at runtime.

kuncevic commented 8 years ago

The problem is that I am relying on third party module so to be more specific it is how I instantiate my third party thing:

//That is '/model/something' module
interface ISomethingModel extends ISomething, mongoose.Document { }    
export var Something = mongoose.model<ISomethingModel>('Something', somethingSchema);  // that is what I meant by NotReallySomehting

ISomething - my interface contained just my custom properties of a model.

export interface ISomething {
    field1: string;
    field2: string;
};

mongoose.Document interface contained operations on a model which is third party coming from https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/mongoose/mongoose.d.ts.

//That is Something from '/mock/something' module:
export class Something implements ISomething {
    public static findOne(something: any, next:Function) {
        //custom implementation to mock 'findOne' method from 'mongoose.Document'
    }
}

That is how I am replacing my imports manually from calling module. If i need my implementation I am using: import { Something } from '../mock/something'; In case I need full functional I am using this: import { Something } from '../model/something';

remojansen commented 8 years ago

Is

class Something implements ISomething { //...

the mock and

var Something = mongoose.model<ISomethingModel>('Something2', somethingSchema); 

the real implementation?

kuncevic commented 8 years ago

@remojansen yes that is correct I just updated with more details

remojansen commented 8 years ago

The problem

So we have a plain object. The interface of the object is:

interface ISomething {
  /// 
}

And its class is:

export class Something implements ISomething {
  public static findOne(something: any, next:Function) {
        //there is similar method on mongoose.Document so I do custom implementation of it here
        //do findOne
    }
}

The method findOne is part of the mongoose Model. So it looks to me like you want ISomething to behave as a Model.

The problem is that then you are using ISomething to create a model:

export var Something = mongoose.model<ISomething>('Something2', somethingSchema); 

You are passing the ISomething to mongoose.model but mongoose is expecting a Document not a model. Both the mock and the real implementation need to implement the same interface.

The solution

Declare ISomething as a Document:

interface ISomething extends mongoose.Document {
  /// 
}

Declare ISomethingModel as a Model:

interface ISomethingModel extends mongoose.model<ISomething>{}

Create a Model using the ISomething Document.

var _model = mongoose.model<ISomething>('Something2', somethingSchema); 

class SomethingModel implements ISomethingModel {
   public static findOne(something: any, next:Function) {
        // USE_model HERE
    }
}

Create a mock of the model ISomethingModel:

class SomethingModelMock implements ISomethingModel {
 public static findOne(something: any, next:Function) {
        // mock _model response here
    }
}

Create your InversifyJS config for the app ans test:

if (app.get('env') === 'dev') {
    kernel.bind(new TypeInstance<ISomethingModel>('ISomethingModel', SomethingModel));
} 
else if (app.get('env') === 'stg') {
     kernel.bind(new TypeInstance<ISomethingModel>('ISomethingModel', SomethingModelMock ));
}

Note: I haven't test this solution but based on the moongose type definitions and an example that I found online I believe it should work.

Tip!

I would recommend to have two inversify.config.ts files. You can have inversify.config.ts in your source directory:

kernel.bind(new TypeInstance<ISomething>('ISomething', Something));

And inversify.test.config.ts in your test directory:

 kernel.bind(new TypeInstance<ISomething>('ISomething', NotReallySomething));

Hope it helps, and please ask again if you have more problems :four_leaf_clover:

kuncevic commented 8 years ago

@remojansen Thanks for help and the Tip is great!

I think it is turned out that this:

interface ISomethingModel extends mongoose.model<ISomething>{}

should be this with Model capital, otherwise got an error.:

interface ISomethingModel extends mongoose.Model<ISomething>{}

Then when I changed the interfaces like per example I we got an error in both cases when implements ISomethingModel

class SomethingModelMock implements ISomethingModel {
 public static findOne(something: any, next:Function) {
        // mock _model response here
    }
}

Error saying that I need to implement aggregate... that is from mongoose.d.ts https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/mongoose/mongoose.d.ts

export interface Model<T extends Document> extends NodeJS.EventEmitter {   
    aggregate(...aggregations: Object[]): Aggregate<T[]>;
    aggregate(aggregation: Object, callback: (err: any, res: T[]) => void): Promise<T[]>;
    aggregate(aggregation1: Object, aggregation2: Object, callback: (err: any, res: T[]) => void): Promise<T[]>;
    aggregate(aggregation1: Object, aggregation2: Object, aggregation3: Object, callback: (err: any, res: T[]) => void): Promise<T[]>;
}

In normal scenario for my case I do not really need to change/override any mongoose stuff

class SomethingModel implements ISomethingModel {
   //nothing custom here
}

But only in second case I need to do changes

class SomethingModelMock implements ISomethingModel {
 public static findOne(something: any, next:Function) {
        // mock _model response here
    }
}

So how to resolve aggregate not implemented not sure yet.

remojansen commented 8 years ago

I think I know what is the problem now. We have the ISomething document:

interface ISomething extends mongoose.Document {
  /// 
}

The SomethingModel:

var somethingModel = mongoose.model<ISomething>('Something2', somethingSchema); 

And the something model mock:

class SomethingModelMock implements ISomethingModel {
 public static findOne(something: any, next:Function) {
        // mock _model response here
    }
}

This is a problem because:

InversifyJS needs classes and is not able to work with instances yet. This is going to be supported in InversifyJS 2.x:

if (app.get('env') === 'dev') {
    kernel.bind<ISomethingModel>('ISomethingModel').toValue(somethingModel);
} 
else if (app.get('env') === 'stg') {
     kernel.bind<ISomethingModel>('ISomethingModel').to(SomethingModelMock);
}

I found a class wrapper for mongoose maybe that can help you to declare somethingModel as a class. Then you will be able to resolve it.

remojansen commented 8 years ago

Hi, the feature that you needed: toValue has been merged into master by https://github.com/inversify/InversifyJS/pull/97. Over the next few weeks we are going to try to make 2.0 stable please keep an eye in the roadmap for updates.