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

Sub-class injection broken for flash 6 #38

Open
jkringen opened this issue Apr 9, 2015 · 1 comment
Open

Sub-class injection broken for flash 6 #38

jkringen opened this issue Apr 9, 2015 · 1 comment

Comments

@jkringen
Copy link

jkringen commented Apr 9, 2015

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).

@dpeek
Copy link
Contributor

dpeek commented Jul 23, 2015

TBH I'm surprised that minject works in AVM1 at all! If you're still interested in fixing it, could you test if the problem still exists in v2 rc1 (which we just released). If it is, a pull request with a failing unit test would be awesome (the unit tests still run in AVM1 believe it or not)

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

2 participants