can/observe/attributes plugin does not properly inherit the attributes property

[rjgotten]

See: https://github.com/bitovi/canjs/blob/1aac092f5df252582d8318df0d6d2ef1707088d9/observe/attributes/attributes.js#L213-L223

Currently the attributes property is not being merged during inheritance, whereas the convert and serialize property are.

Having the attributes collection not inherit over flies in the face of OO design; if you create an inherited model, you want it to inherit from its base. You don't want to re-specify the core of the class, i.e. , the attributes and their type mappings, every time you create a layer of inheritance.

Introducing this rift in behavior also gives rise to a very subtle class of application bug where converters for attributes mysteriously 'go missing', with no obvious reason as to the reason. After all; the convert and serialize members are properly inherited and present, and all the attributes will be present as well (thanks to the default behavior of can.Observe processing and observing all child properties on the passed in data object). However, as the attribute mapping is not inherited, the calls to the converters simply never happen.

If this inheritance behavior is by design, then it is a very poor design choice.

[justinbmeyer]

It used to be this way (likely JMVC 3.0), but we changed it because it was very hard to turn off attributes in inheriting constructor functions.

[rjgotten]

Not 'very hard' at all: From the looks of things, the only required change to the can/observe/attributes plugin is that the default type converter act as a direct passthrough when its type parameter is null.

The above change would finish completely wiring up the plugin to ignore a null value for an attribute type in its converters and serializers, setting them to act as the default direct value passthrough. If can.extend indeed completely mirrors jQuery.extend, then only undefined values are skipped when merging object literals; null values should still be merged over any previously established values.

Provided that the above holds, a very simple pattern allows you to reset the converter and serializer for an attribute by passing null as the attribute type during inheritance:

var BaseObserve = can.Observe({
  attributes : { "value" : "boolean" }
})

var InheritedObserve = BaseObserve({
  attributes : { "value" : null }
});

var base = new BaseObserve({ "value" : "false" });
console.log( typeof base.value ); // "boolean"

var inh = new InheritedObserve({ "value" : "false" });
console.log( typeof inh.value ); // "string" instead of "boolean"

Sure, you won't be able to get rid of the attribute entirely, but resetting its converter and serializer effectively does the same thing. All data members, also those not marked up explicitly as attributes, are still made observable by can.Observe and by default it will still serialize all those observable members back into a raw object.

From a less pragmatic and more theoretical standpoint, some thoughts you may want to consider are:

1. how messed up your application architecture has to be to require undefining part of a class for inheritance,
2. if you should care to support that kind of crazy scenario and all the cans of worms* that come with it, and
3. if, from an implementation guidance point of view, it would perhaps be wiser to steer your library consumers away from adopting that kind of design...

*) sound polymorphism goes out the door, for one

[justinbmeyer]

Define will inherit by default. Use that in 2.1 going forward.