massive-oss / minject Goto Github PK

Injecting components into each other causes the app to crash.

class Test1
{
        @inject public var test:Test2;
        public function new() {}
}

class Test2
{
        @inject public var test:Test1;
        public function new() {}
}

class Test
{
        static function main()
        {
                var injector = new minject.Injector();
                injector.mapSingleton(Test1);
                injector.mapSingleton(Test2);
                injector.getInstance(Test1);
        }
}

I'm having some issues mapping typedefs that define an anonymous structure, like typedef UAProvider = { var userAgent:String } because of a compilation error when calling Context.typeof(expr).

I'm trying to fix it to support anonymous structures but i noticed a commend in the followType method that explicitly avoids to try to resolve the TAnonymous type.

What's the rationale behind that? Is there a reason why that is disallowed? The mapping syntax would be much easier and maintainable if i could say injector.map(MyAnonTypeDef) instead of manually writing a string that describes the fields.

Because getInstance() will instantiate an instance if it doesn't have an existing value, it doesn't make sense for non-class values that we have rules for.

The current workaround I have is:

if ( context.injector.hasRule(Int,"sessionExpiry") )
    expiry = context.injector.getRule( Int, "sessionExpiry" ).getResponse(null);

Which isn't too bad. It would be nice if the parameter for getResponse() was optional though, as it already has a sensible fallback when the parameter is null.

If you want a more complete solution, an alternative might be:

injector.getValue( Int, "sessionExpiry" );

Which would search for a mapped value, but and either return null (or throw an error) if no value exists, rather than attempting to instantiate a class.

FYI, since this is Flash 6 specific I realize no one may care about this issue, but our company is, unfortunately, stuck with Flash 6 as a runtime environment for the meantime. We have utilized minject quite heavily in our javascript code base and now are migrating the same framework to our flash based systems.

We noticed this week that injection was never performed on any super classes at all. For example:

class BaseClass {
    @inject public var thing:Thing;
    @inject public var otherThing:OtherThing;
}
class AnotherClass extends BaseClass() {
}
injector.mapClass(BaseClass, AnotherClass);

**This code is not complete code at all, just an example of a parent/child class scenario

At runtime, when a AnotherClass instance is created, we noticed that the 2 fields defined in the BaseClass class were not injected at all for some reason. We quickly realized that this error was tied to the fact that the @Inject variables were in the super class of the class that was being injected. After looking into it a bit more the cause of the problem is that, when running in Flash 6, the Type.getSuperClass() method always returns null, so the Injector.getFields() method never traverses up to the super class to get the metadata properly, causing any injection points in the super class(es) to not be processed.

We do have a fix for this that we implemented by modifying a local copy of minject (based on 1.5.2 since 1.6.0 appears to not work at all for us). We altered the Injector.getFields() method to, unfortunately, rely on RTTI XML metadata (generated by tagging your sub-classes with the @:rtti annotation) to be able to traverse the class hierarchy.

Obviously Flash 6 is archaic, so this fix may only be of interest to us, but if you would like the fix we have, we can supply that for you. Here is the new Injector.getFields() function that now works for Flash 6:

function getFields(type:Class<Dynamic>) {
    var meta = {};
    while (type != null) {
        if (type != null) {
            var typeMeta = haxe.rtti.Meta.getFields(type);
            for (field in Reflect.fields(typeMeta)) {
                Reflect.setField(meta, field, Reflect.field(typeMeta, field));
            }
            var rttiData = Reflect.getProperty(type, "__rtti");
            if (rttiData != null) {
                var xmlData:Xml = Xml.parse(Std.string(rttiData));
                var superClassDef:Iterator<Xml> = xmlData.firstElement().elementsNamed("extends");
                if (superClassDef.hasNext()) {
                    var superClassNode:Xml = superClassDef.next();
                    var superType = untyped __eval__(superClassNode.get("path"));
                    Type.createInstance(superType, []);
                    type = cast superType;
                }
            } else {
                if (Reflect.getProperty(type, "__super__")) {
                    type = Type.getSuperClass(type);
                } else {
                    break;
                }
            }
        }
    }
    return meta;
}

This approach does require that all sub-classes be tagged with the @:rtti annotation, which seems acceptable when compared to fixing the underlying flash 6 problem itself (which seems to be a Haxe specific bug with Flash 6 due to differences with class definition structures between flash 6 & 7).

following line https://github.com/massiveinteractive/minject/blob/master/src/minject/point/PropertyInjectionPoint.hx#L48 is translated into javascript:

if(injection == null) throw "Injector is missing a rule to handle injection into property \"" + this.name + "\" " + ("of object \"" + Std.string(target) + "\". Target dependency: \"" + this.type + "\", named \"" + this.injectionName + "\"");

In case Std.string() throws error itself then the origin exception is not fired and is hard to trace. It was a case of missing command mapping in our case

Seems that haxe 3.1.1 has changed something there, since haxe 3.0 compiles just fine.

injector.mapValue(SomeClass, null);

creates mapping but injection fails due to:
line: 46 if (injection == null)
in minject / src / minject / point / PropertyInjectionPoint.hx

changing it to
if( injector.hasMapping(Type.resolveClass(propertyType), injectionName) == false);

fixes it.

Unless its intended behaviour, but i think it should be valid to map nulls.

Since V2 uses macros if you were to call Injector methods indirectly the following no longer works.

var type:Class<Dynamic> = Class1;
injector.mapValue(type, null);

Or alternatively

var mapper:MyMapper = new MyMapper();
mapper.mapValue(Class1, new Class1());

I'm unable to run the test harness due to a missing dependencies. mcore 1,0,0 and mdata 0,9,0 are empty.

Injector.hx:27: characters 0-24 : Class not found : mdata.Dictionary

Because the addMeta method is registered as a build macro of Injector, meaning that if Injector is cached by the compilation server and no need to be bulit again, minject will fail at runtime because the required metas are absent. (ufront/ufront-mvc#53, HaxeFoundation/haxe#4892)

I have fixed that by moving the onGenerate registration to extraParams.hxml (source). That works well but not optimal, because onGenerate re-processes all types every time. So I further improved it to use tink_syntaxhub, making it process only changed/uncached types. And the source is here.

Please let me know which solution you prefer and I will make the PR correspondingly.

Considering the class

class A implements IOne implements ITwo {
...
}

With the following mapping:

injector.map(IOne).toSingleton(A);
injector.map(ITwo).toSingleton(A);

I will get two separate instances of class A used for the respective injection points (at least in JS).
I would have expected to have one single instance to be provided whenever one of the interfaces is requested.

I can easily switch to a toValue() type of mapping to solve my case, I was just wondering if this is the expected behaviour and my assumption is totally wrong.

We've had to drop back to v1.5.2 as when using v1.6.0 via mmvc we have come across injections which have not been executed, and no error is thrown until you try to access the property and it's null. In our case in onRegister.

Code to reproduce:

class Parent {
    @inject public var name:String;
}
class Child extends Parent {
    public var postRanCorrectly = false;
    @post public function afterInjection() {
        postRanCorrectly = (name!=null);
    }
}

var injector = new minject.Injector();
injector.map( String ).toValue( "Jason" );
var child = injector.instantiate( Child );
Assert.isTrue( child.postRanCorrectly ); // FAILS!
Assert.equals( "Jason", child.name ); // yet this is okay...

Haven't dived into the code yet to verify if what I've said in the title is in fact the root of the problem. But it probably is.

The current RC version is causing some problems with haxelib that it won't get identified as the newest version. In other words, haxelib install minject will give you v1.6 instead of v.2.0.0-rc1.

Have been using minject v2 for some time with Ufront and everything seems working well so far.

src/mcore/util/Reflection.hx:31: characters 0-41 : Class not found : mcore.exception.ArgumentException

<3

One of the problems with using dependency injection is that the Haxe compiler likes to eliminate the constructors (and other parts) of injected classes, as most of them are never explicitly instantiated (created with Type.createInstance instead).

One possible macro based solution:

macro function mapClass(theClass:Expr)
function _mapClass(theClass:Class<Dynamic>);

// so, when you call mapClass it adds @:keep to theClass and generates
_mapClass(theClass);

The problem with this approach being that anyone calling the map methods (i.e. mmvc) need to do so in macro time too so that the types get through.

I'm open to better suggestions :)

If i have two typedef like

typedef IServiceOne = IService<One>
typedef IServiceTwo = IService<Two>

with IService being an interface, and i try to map them to concrete classes, like

injector.map(IServiceOne).map(ServiceOne);
injector.map(IServiceTwo).map(ServiceTwo);

I'd get a warning because the mapping [IService -> ServiceTwo] is trying to override the mapping [IService -> ServiceOne]

It looks like the type parameter of my interface is not taken in consideration when creating the mapping.

[Workaround for this is to have IServiceOne and IServiceTwo as interfaces that extend IService instead of typedefs.]

Hey

In swift suspenders the following would work, which I really liked.

class Foo {
}

var foo = injector.getInstance(Foo); //I've never mapped Foo
MatcherAssert.assertThat(foo, IsNull.notNullValue());

It cuts out a decent amount of boiler plate code.

Would you want this in minject?

Allow an optional PropertyInjectionPoint.

Similar to how method injection points allow optional parameters to be skipped, check if a variable is Null<T>^, mark it as optional in the RTTI, and if there is no result, don't throw an error when applying the injection.

^ If this isn't explicit enough, perhaps some metadata?

Compiling under haxe 3 RC2 gives the following error:
/usr/lib/haxe/lib/minject/1,1,0/minject/RTTI.hx:137: characters 9-19 : Arguments expected

In haxe.macro.Type the enum VarAccess (line 129) has changed from this:
enum VarAccess {
AccNormal;
AccNo;
AccNever;
AccResolve;
AccCall( m : String );
AccInline;
AccRequire( r : String, ?msg : String );
}

To this:
enum VarAccess {
AccNormal;
AccNo;
AccNever;
AccResolve;
AccCall;
AccInline;
AccRequire( r : String, ?msg : String );
}

Because ObjectMaps do not use weak references and CPP has no weakmap(HaxeFoundation/haxe#3200) minject leaks for all objects injected or instantiated in CPP. I was able to verify this in GlowCode, a memory profiler for windows.

The ObjectMap is used in attendToInjectees. I'd prefer removing it altogether as I never run into the defensive use-case I think it was intended for. Or maybe you'd prefer a removal/exception for CPP? If you want removal for CPP only I have a branch ready.

Using mmvc in a Haxe 3.1.0 project yields the following errors:

..\haxe-3.1.0-win\lib\minject/1,2,3/mcore/exception/Exception.hx:130: characters 9-15 : Identifier 'Lambda' is not part of enum haxe.StackItem
..\haxe-3.1.0-win\lib\minject/1,2,3/mcore/exception/Exception.hx:130: characters 9-18 : Expected constructor for enum haxe.StackItem

All is fine with Haxe 3.0.

Hello Dynamics,
--dead-code-elimination (DCE) has the bad habit to remove the constructors of the classes instantiated through Type.createInstance(). I wonder if you can add the @:keep tag to the constructor of the classes registered in your API using macros.
What do you think?

Injector.getTypeRule() takes a String representing the type. Sometimes this string is supplied by InjectorMacros.getType() (which includes type parameters) and sometimes it is supplied by Type.getClassName (which does not).

This can lead to the type names on mapped rules not matching the type names on injection points, and can cause errors.

Some options:

1. Not support type parameters.
   This means the fix will be as simple as stripping them from the various places in InjectorMacros.
2. Support type parameters.
   This means changing _mapClass(), _mapSingletonOf(), _getInstance() to use a macro-powered type name instead of a class, which will be easy. We'd also need to change _unmap() to be macro powered, also pretty easy.

Possible issues with supporting type parameters:

• Unification won't work, exact type matches only. Eg. An injection point for Array<Dynamic> won't match a rule for Array<Int>. Which I'd say violates the rule of least surprise somewhat.

Which way forward? Personally I'd prefer to support type parameters, even with the unification caveat. But you decide. I'm happy to provide a pull request.

In php i get this throw for weakmaps. throw "Not implemented for this platform";
}

On the JS target, when you attempt to inject a class that hasn't been mapped you get a runtime exception with no meaningful error indicating which class failed to inject.

I can see there's error handling within Injector to print out the class name if getInstance() fails to find a mapping e.g. here,
and also PropertyInjectionPoint prints out the class name if applyInjection fails e.g. here, however neither of these are thrown and instead a runtime exception is raised as seen here (even when building a debug build):

Currently it's tedious figuring out which mappings are missing. If these were thrown as intended it would become trivial to add missing mappings.

The rc.1 release in haxelib can't be used because it does not recognise the imported classes when compiling.
Maybe release a working rc.2?