Fix/module proxy multiple inheritance

[alexjreich]

Pretty straightforward recursion to include multiple levels of inheritance for module proxy methods.

@loppear the thing I wasn't sure about what the test fixes - some of those config tests were broken but probably from before? I fixed but could use your eyes.

[loppear]

Threw me for a bit - this is just fixing inheritance, not introducing multiple inheritance. Tests pass on master so I'm worried if this broke one about config initially, esp with that special-casing of a bunch of property names in the method.

Will take a closer review look later today.

[alexjreich]

Yeah, true, sorry abut the confusion. There probably is a more elegant way to handle those hard coded names (though we do the same thing with constructor) and it probably is harmless to bind log and degregister To events anyhow.

In my testing, everything works as expected except with the @autobind Decorator - its obviously wrapping a method in a way we don’t test for yet. Probably some other edge cases out there would be good to catch before publishing.

[loppear]

Doesn't work with autobind because that actually creates a getter/setter property, so its skipped. So let's go back a step, presumably the initial bug for this PR is that you can't use superclass methods as proxy methods - that's a bug for sure, and presumably I had some reason for hasOwnProperty rather than letting it pick up all the property names... Do you recall what issues came up with ModuleProxy binding to getters, config, log, etc that you added that part in?

[alexjreich]

@loppear the current test obj[prop] instanceOf Function doesn't work for getters, because it will access the getter before instantiation. This most obviously breaks anything extending HasModels, as this.models getter with throw an exception when use is run. Not so sure about config other than it broke some tests in NxusModule, not totally sure why.

[loppear]

Yeah I see, doesn't break HasModels generally but breaks on ViewController.get model that returns a value from this.models which doesn't exist yet. I don't see a way to distinguish between autobind getters and that kind, we could either require that module getters be safe to call during construction (nah) or just catch the error of accessing unsafe getters and don't treat them as proxyable (nor any that return a non-function, like config). I've separated that check from the _ check, so if we have need of an expensive unsafe getter we can at least make it private and avoid checking it here.

[alexjreich]

@loppear sounds good! I like the approach. I’ll try on a couple of apps and let you know if I run into bugs.