webcomponents / polyfills

Web Components Polyfills
BSD 3-Clause "New" or "Revised" License
1.13k stars 165 forks source link

[scoped-custom-element-registry] toggleAttribute should only trigger attributeChangedCallback when the attribute is actually toggled #556

Open jessevanassen opened 10 months ago

jessevanassen commented 10 months ago

Description

According to the toggleAttribute(qualifiedName, force) spec, toggling an attribute should only result in a change if the attribute actually changes. In other words, if the attribute is already present and force is true or if the attribute is not present and force is false, the attribute shouldn't change.

When the polyfill is loaded, the attribute is always changed, resulting in the attributeChangedCallback firing too frequently.

This is also inconsistent with how it works out of the box in the latest versions of Chrome, Firefox and Edge.

It is especially problematic, because it breaks the spec for all web components on the page, not just components that use the scoped registry. In our case, it lead to an unrelated piece of code behaving differently when the polyfill was loaded elsewhere.

Example

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <script src="https://unpkg.com/@webcomponents/scoped-custom-element-registry@0.0.9/src/scoped-custom-element-registry.js"></script>
    </head>
    <body>
        <script type="text/javascript">
            customElements.define('my-element', class MyElement extends HTMLElement {
                static get observedAttributes() {
                    return ['param'];
                }

                attributeChangedCallback(name, oldValue, newValue) {
                    console.log('attributeChangedCallback', { name, oldValue, newValue });
                }
            });

            const element = document.createElement('my-element');
            document.body.append(element);

            // This should log: first setting of param
            element.setAttribute('param', '');

            // This should log: removing the param
            element.toggleAttribute('param', false);

            // This shouldn't log: parameter is not present
            element.toggleAttribute('param', false);

            // This should log: adding the parameter 
            element.toggleAttribute('param', true);

            // This shouldn't log: parameter is already present
            element.toggleAttribute('param', true);
        </script>
    </body>
</html>

Expected behavior

attributeChangedCallback is only called when the attribute is actually toggled

Actual behavior

attributeChangedCallback is also called when the attribute presence remains the same

Version

0.0.9

Browsers affected

(These are the only browsers I can test)