Boolean attributes written in the kebab case are overridden by common observable attributes

[null0rUndefined]

When we have a parent class with only boolean attributes defined, the define method will concat boolean and observed attributes. This means that when we extend the parent class without overriding any boolean or observed attributes, the new class will contain equal values ​​in the boolean and observed attribute arrays.

When the define method is called for a new class, it defines boolean attribute getters and then replaces it with normal observed attribute getters due to incorrect check of existing attribute getters in kebab-case.

class Parent extends HyperHTMLElement {
    static get booleanAttributes() {
        return ['kebab-case'];
    }
}

Parent.define('parent-class');
console.log(Parent.booleanAttributes) // ['kebab-case']
console.log(Parent.observedAttributes) // ['kebab-case']

class Child extends Parent {};
Child.define('child-class');

const child = document.createElement('child-class');
child.kebabCase = 'any value';
console.log(child.kebabCase) // 'any value' instead of true

The problem is that the define method checks the attribute in the prototype using its name in the kebab case, but defines a getter in the camel case. This leads to the problem that a defined getter for a boolean attribute can be overridden by a normal getter for an observed attribute.

[WebReflection]

I am not sure I am following here ... to start with, the extend is supposed to have indeed those observedAttributes, and observedAttributes must be present to observe ... well ... attributes.

the code then differentiates between boolean and regular, and if you don't want to inherit observed attributes (which is correct/expected) you can override the getter, right?

boolean attributes and kebab-case are also kinda unrelated ... so, could you please elaborate a bit more what is the issue with kebab-case only and, eventually, epxlain what is the issue with boolean attributes?

[null0rUndefined]

Good afternoon!

As for the previous example with class inheritance, I was just trying to show a real user case in which I caught this bug.

The message here is that the hyperhtml-element behaves differently when adding getters for boolean attributes: example A:

class ClassA extends HyperHTMLElement {
    static get booleanAttributes() {
        return ['attribute'];
    }

    static get observedAttributes() {
        return ['attribute'];
    }
}

ClassA.define('class-a');
const classA = document.createElement('class-a');
classA.attribute = 'any value';
console.log(classA.attribute) // true, because it is a boolean attribute

example B, but with the kebab-case attribute:

class ClassB extends HyperHTMLElement {
    static get booleanAttributes() {
        return ['kebab-case'];
    }

    static get observedAttributes() {
        return ['kebab-case']; 
/* Adding the same values to booleanAttributes and observedAttributes is not a frequent use case.
We need it here to bring us closer to the data set that is obtained when inheriting classes */
    }
}

ClassB.define('class-b');
const classB = document.createElement('class-b');
classB.kebabCase = 'any value';
console.log(classB.kebabCase) // 'any value' instead of true

The only difference is that in the second example we have attribute defined in kebab-case. As I mentioned before:

The problem is that the define method checks the attribute in the prototype using its name in the kebab case, but defines a getter in the camel case. This leads to the problem that a defined getter for a boolean attribute can be overridden by a normal getter for an observed attribute.

86e09b36fdc50723fcf185586c68914153d3172d

Regarding the redefinition of getters, I agree that in any case they can be redefined and everything will work, but it seems strange to me that we declare only booleanAttributes without observedAttributes (in the real-life example) and in the inherited class we will have to redefine getters for each boolean attribute so that it works as a boolean (as expected) instead of the usual one.

[WebReflection]

It makes no sense to define the same attribute in both places, so I still don't understand the issue. Can you please provide an issue using only the booleanAttributes ?

[WebReflection]

To clarify, using the same attribute in both places is just not supported, so this is looking like a wontfix/not-needed feature.

You can, however, returns, overwrite, anything inherited, in case it's needed, but same attribute name as both boolean and observed is not going to happen here.

[null0rUndefined]

The problem with attributes in kebab-case appears only during inheritance:

class Toolbar extends HyperHTMLElement {
    static get booleanAttributes() {
        return ['show-tooltips'];
    }

    shouldShowTooltip() {
        return this.showTooltips === true;
    }
}

Toolbar.define('class-toolbar');
const toolbar = document.createElement('class-toolbar');

toolbar.showTooltips = true;
console.log(toolbar.showTooltips); // true
console.log(typeof toolbar.showTooltips); // boolean

toolbar.showTooltips = 'any value';
console.log(toolbar.showTooltips); // true
console.log(typeof toolbar.showTooltips); // boolean, because it is a boolean attribute

console.log(toolbar.shouldShowTooltip()); // true

// after inheritance, getters begin to return completely different values
class ExtendedToolbar extends Toolbar {
    // define nothing
    /* or we can define same boolean attributes, doesn't matter
        static get booleanAttributes() {
            return ['show-tooltips'];
        }
    */
}

ExtendedToolbar.define('class-extended-toolbar');
const extendedToolbar = document.createElement('class-extended-toolbar');

extendedToolbar.showTooltips = true;
console.log(extendedToolbar.showTooltips); // 'true'
console.log(typeof extendedToolbar.showTooltips); // string

extendedToolbar.showTooltips = 'any value';
console.log(extendedToolbar.showTooltips); // 'any value'
console.log(typeof extendedToolbar.showTooltips); // string, but should be boolean

console.log(extendedToolbar.shouldShowTooltip()); // will always be false, because any string value will never be true

I expect the class to have the same getters for boolean attributes after inheritance and return true or false, not strings

[WebReflection]

OK, thank you for clarifying this part. It's indeed unexpected, and I'll try to make it right.

[WebReflection]

This use case has been fixed.

[null0rUndefined]

@WebReflection Hello!

Thank you for the fix, but unfortunately it doesn't fix the root cause, but only one of the consequences. Here's another example that can reproduce the real problem:

class ClassA extends HyperHTMLElement {
    static get observedAttributes() {
        return ['attribute'];
    }

    get attribute () {
        return 'Predefenied getter';
    }
}

ClassA.define('class-a');
const classA = document.createElement('class-a');
console.log(classA.attribute); // 'Predefenied getter'

class ClassB extends HyperHTMLElement {
    static get observedAttributes() {
        return ['kebab-case'];
    }

    get kebabCase () {
        return 'Predefined getter';
    }
}

ClassB.define('class-b');
const classB = document.createElement('class-b');
console.log(classB.kebabCase); // null, it was overwritten

The same problem can be reproduced with class inheritance

The problem is that the define method checks the attribute in the prototype using its name in the kebab case, but defines a getter in the camel case. This leads to the problem that a predefined getter can be overridden by a getter for an observed attribute.

I have a pull request that fixes this issue: #74

[WebReflection]

I saw that yesterday while fixing the Boolean issue. One fix at the time 😁

[WebReflection]

It's up and running.

[null0rUndefined]

Thank you!