superCalls on custom setters don't always work on inherited properties

[tkrugg]

If widget Child inherits from Mother a property "p", I don't seem to be able to do a superCall on the custom setter of "p" inside Child, if such a custom setter didn't exist in Mother.

[wkeese]

Probably this is working, because we are doing it in other code, for example deliteful/Select.js does this:

_setSelectionModeAttr: dcl.superCall(function (sup) {
    // Override of the setter from delite/Selection to forbid the values
    // "none" and "radio"
    return function (value) {
        if (value !== "single" && value !== "multiple") {
            throw new TypeError("'" + value +
                "' not supported for selectionMode; keeping the previous value of '" +
                this.selectionMode + "'");
        } else {
            this._set("selectionMode", value);
        }
        sup.call(this, value);
    };
})

If you can come up with a test case that fails though, I'll take a look.

[tkrugg]

I've seen that one. It doesn't fit my description. Select inherits selectionMode from Selection, but Selection itself implements a _setSelectionModeAttr method.

_setSelectionModeAttr: function (value) {
    if (value !== "none" && value !== "single" && value !== "multiple" && value !== "radio") {
        throw new TypeError("selectionMode invalid value");
    }
    if (value !== this.selectionMode) {
        this._set("selectionMode", value);
        if (value === "none") {
            this.selectedItems = null;
        } else if ((value === "single" || value === "radio") && this.selectedItem) {
            this.selectedItems = [this.selectedItem];
        }
    }
},

If that didn't work, it would be a dcl issue for sure.

This issue is precisely for the case when:

such a custom setter didn't exist in Mother.

[AdrianVasiliu]

I guess this would do the trick in both cases (regardless of whether the superclass has a custom setter for the property p):

_setPAttr: dcl.superCall(function (sup) {
    return function (value) {
        ...
        if (sup) {
            sup.call(this, value);
        } else {
            this._set("p", value);
        }
    };
}),

Not sure if it would be feasible (and desirable) to get this somehow automatically handled (by dcl), that is having a sup function automatically generated when no custom setter exists on the superclass(es).

[wkeese]

It seems strange to me even to support the above syntax. In other words, it seems strange that you want to use dcl.superCall() when there is no such method in the superclasses. Also, I don't think this has anything to do with delite; it's just a question of how dcl works.

[AdrianVasiliu]

I also think we can live with what we have. That said, it may have been nice if the implementation of a custom setter for an inherited property didn't need to be different depending on whether the parent class has a custom setter - the presence of the custom setter on the parent looks as an implementation detail which, in an ideal world, shouldn't impact the way subclasses are coded. Anyway, rather a dcl issue, I agree. My cts.

[AdrianVasiliu]

Just to add:

it seems strange that you want to use dcl.superCall() when there is no such method in the superclasses.

I think the situation is different than for method overrides. To stay with the example of Selection.selectionMode, the reader of the API doc of delite/Selection doesn't even know whether a custom setter exists for this property. The custom setter is not part of the public API. That's why it's more painful for implementors of custom setters for inherited properties to decide how to code it than it is when implementing an override of a public documented method.

Also, it seems to me that the current picture implies:

• From one release to another, introducing or removing a custom setter on a widget (or any Stateful subclass) potentially creates an incompatibility for subclasses that have custom setters for inherited properties. Hence, strictly speaking, the addition/removal of custom setters should be documented as incompatibility in release notes...
• Custom setters start with underscore and are treated as private methods however subclasses may need to override them. Maybe it would be better to "promote" custom setters to "protected" (thus loose their underscore prefix), making them part of the public documented API? Just wondering.