Inheritance (again)

[jods4]

Could we reconsider supporting inheritance in behaviors (e.g. custom elements, custom attributes)?
This was brought up (and closed) several times in the past, e.g. #84, #249.
@EisenbergEffect said

I don't think we're going to add this for now. I'm not sure it's a good idea. However, we have provided some options to use mixins to share behavior.

Could you provide more guidance regarding mixins support?

Motivation

In our codebase, several times we define a common base class for similar custom elements or custom attributes, with just slightly different configuration choices.

For example, we have a better <select> control as a custom element, that is instantiated in two flavours: with a local list of items and with an AJAX lookup. The base class defines some common behaviour and options.

This works for us, except that the metadata is not inherited. For example, the base class defines

@bindable 
valueField = 'id';
@bindable
labelField = 'name';

But the @bindable annotation is lost on descendants and we have to copy-paste them on both the local and AJAX variations.

An analysis of the code shows that the problem in this specific case is that all CustomElement-related metadata is stored in a single object with key metadata.resource.
Because this dictionary exists in the derived class, properties of the base class are totally ignored.

I think that at least this part could be fixed by merging metadata.resource objects from derived to base class before usage by Aurelia.

[jdanyow]

@jods4 would a solution that lifts the @bindable properties from the base class to the derived class fulfill the need or are you looking for something more advanced? For example, reconciling the class-level attributes such as @inject, etc could be difficult/ambiguous. I'm trying to get a handle on the scope.

[jods4]

@jdanyow Somehow, @autoinject / @inject is already working fine for us in derived scenarios.

In our codebase, the only thing that we would need at the moment is have @bindable properties from base class discovered. I guess other people might have more complex needs, but @bindable is the main culprit for us.

[atsu85]

@bindable is the main culprit for us.

same here.

@EisenbergEffect has suggested using decorators, but that takes much more effort compared to base class and if You happen to use TypeScript, then You really start to hate that approach (as TS compiler can't know at compile time what might be added at runtime to the class - so You either need to cast this to any before each use of properties/functions added by the decorator, or mess with interfaces to get type safety...).

[EisenbergEffect]

I'm really apposed to inheritance. That doesn't mean we can't do this. It's likely to take a lower priority for the team though since there are other ways to handle the solution and other features/fixes that are more critical. However...if someone were to submit a PR that made inheritance work...with tests...then there's a good likely hood it would get in pretty quickly.

[jods4]

@EisenbergEffect Can you link some doc regarding the better ways? Thanks!

[EisenbergEffect]

I don't have a doc specifically on that subject. Perhaps search the internet for "using decorators for composition" or something like that.

[Kukks]

@jdanyow @EisenbergEffect I think the bindables from the base not being visible is the main issue and headache. After spending a few months on a big app, it starts being a headache maintaining a bunch of bindable decorators on extended elements after a change.

Can you point us in the right direction to where to start for this change?

[Infuser]

Ok I commented on issue 210 but seems this is a more up to date ticket

Having @bindable work using inheritance I think is very important the only alternative is lots of copy and paste code

If there are some other options could a code example be provided

[Kukks]

@Infuser I found this decorate in one of the ui bridges. Maybe it can help us in some way?
https://github.com/aurelia-ui-toolkits/aurelia-kendoui-bridge/blob/master/src/common/decorators.js

[Infuser]

Thanks interesting I see people talking about it in this post not sure how to use it yet though need more time

aurelia-ui-toolkits/aurelia-kendoui-bridge#232

[EisenbergEffect]

I highly recommend to use decorators. You can basically use them to meta-program your JS classes. They can dynamically add shared behavior. Because they are decorators, you can have as many as you need as opposed to inheritance which only allows for one base class. Decorators favor the composition style of OO which gives you a better set of options than inheritance usually.

[Infuser]

Hi Rob

Thanks for the response I am sure you are correct using decorators is a better approach sadly not sure what you mean could please provide a code example for this in relation to the @bindable issue

Thanks in advance

[Infuser]

Well I gave it a try and had no luck using this web site as a guide

https://medium.com/google-developers/exploring-es7-decorators-76ecb65fb841#.g0voh5lni

I get inputbase is not a function

[Infuser]

Ok here is a basic decorator that works with esnext I will try putting some bindables on it later

export function superhero(isSuperHero)
{
return function(target){
target.prototype.isSuperHero = isSuperHero;
}
}

[Infuser]

Finally got a chance to have another look at this ended up with code below seems to work would appreciate any comments/constructive criticism's
import {inject, bindable, bindingMode} from 'aurelia-framework';

export function inputbase()
{
return function(target){
target.prototype.itemIdChanged1 = function itemIdChanged1() {debugger};

 let deco = bindable({
            name: 'itemId1',
            attribute: 'item-id1',
            changeHandler: 'itemIdChanged1',
            defaultBindingMode: bindingMode.oneWay
            }).bind(target);

     deco(target.prototype.constructor, undefined, undefined);

}
}

[jods4]

I have code ready for this, I'll prepare a PR with some explanations.
It's not trivial and although the change is rather small there is one part that will require a bit of review.

[jods4]

@EisenbergEffect I was testing my code and I banged my head again on the metadata inheritance in ES5 code transpiled by TS: aurelia/polyfills#34, microsoft/TypeScript#4266.

TL;DR: if you want to mix class inheritance and metadata in ES5 code generated by TS:

• it won't work at all in IE 10-, don't bother.
• if your browser has native Reflect you are fine.
• if your browser has native WeakMap, you can include CoreJS Reflect polyfill, which is based on it and it will work (unlike Aurelia's own polyfill).
• if your browser has Object.setPrototypeOf, you can replace __extends TS helper method, e.g. with Babel equivalent helper, and it will work.

The last option is probably the best, because it gives you true ES6 class semantics, while the other ones are just work-arounds for metadata specifically (setPrototypeOf is IE11+).

This is not directly-related to the upcoming PR but it is prominently showcased. ☹️
Thoughts?

[EisenbergEffect]

Is there some way that we could create a plugin for Aurelia that enabled inheritance. Things the plugin could do are: provide an alternate Reflect.metadata implementation, replace the __extends helper method if present, etc.

[jods4]

@EisenbergEffect The best thing we could do is try to convince TypeScript guys (again) that in 2017 having an ES6 compliant code-gen in 90% of the browsers and have an unfixable edge-case in 10% is better than being consistently incorrect. Or ask them to provide an easy opt-in (or better opt-out!) flag to turn on/off compliance. This is even worse now with --importHelpers as overriding __extends just got harder microsoft/tslib#14. It is also similarly hard with webpack, which doesn't expose globals in this object.

Other than that, Aurelia could change its Reflect.metadata implementation to use WeakMap aurelia/polyfills#34. It would fix the problem for IE11+ (unfixable in IE 10- anyway).

Replacing __extends is not reliably doable.

[jods4]

@EisenbergEffect Great news, it seems MS might finally change their mind and have an ES-compliant __extends in capable browsers (IE11+), while retaining the current (but slightly different) behavior in IE10-.
See microsoft/TypeScript#12488

[tjad]

It's 6 months down the line, this feature is still not implemented. I agree on the flexibility of composition > inheritance, and understand that by using composition one can achieve essentially the same result.
I however do not agree with the complexity required to create simple components that could get away with inheritance. If I understand that my model is really a case that doesn't require composition, I wouldn't want to have to deal with the complexity of creating annotations and composing my components with these decorators. Like why use a scoped rifle when shooting close range?

@jods4 What ever came of your "Great news"

EDIT: Yes, it's the @bindable decorator that is most appealing. I have a base component which offers a generic view that is configurable. I'd like to extend the component with hard coded configurations in a minimalistic manner, inheritance suits this approach.

[Kukks]

@tjad Agreed, it is a real headache to keep stumbling into this. As nice as it is to have a preferred approach for how to model the components, we should have the ability to use inheritance for our own tastes.

[tjad]

I see, @jods4 has created a PR which is active
#507

Perhaps this should be closed.

[jods4]

I guess it can be closed when the PR is merged! The PR has been open for a long time now.

[EisenbergEffect]

You and I need to chat so I can get a full rundown on this and current transpiler limitations, if any. I'll try to catch you online in the next couple of days.

…

On May 23, 2017 12:13 PM, "jods" ***@***.***> wrote: I guess it can be closed when the PR is merged! The PR has been open for a *long* time now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#463 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIBnSdn87qf7qw8G1mhCQFq_k-MTfH2ks5r8y-1gaJpZM4J992I> .

[timfish]

This can be closed. I've just tested it working in the latest release!

[bmcdaniels]

Ran into a situation a few days ago where an easy solution was to bind to a base class in my custom components. Was pleasantly surprised to see such a long standing issue solved just in time for me to utilize this functionality. Great work!