[scoped-custom-element-registry] Toggle attribute should only trigger attributechangedcallback on change

packages/scoped-custom-element-registry/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- ## Unreleased -->
 
+## [Unreleased]
+
+### Fixed
+
+- `toggleAttribute` only triggers `attributeChangedCallback` when the attribute actually changes (#556).
+
 ## [0.0.9] - 2023-03-30
 
 - Update dependencies ([#542](https://github.com/webcomponents/polyfills/pull/542))

packages/scoped-custom-element-registry/src/scoped-custom-element-registry.js

@@ -354,7 +354,10 @@ if (!ShadowRoot.prototype.createElement) {
           const old = this.getAttribute(name);
           toggleAttribute.call(this, name, force);
           const newValue = this.getAttribute(name);
-          attributeChangedCallback.call(this, name, old, newValue);
+
+          if (old !== newValue) {
+            attributeChangedCallback.call(this, name, old, newValue);
+          }
         } else {
           toggleAttribute.call(this, name, force);
         }

packages/scoped-custom-element-registry/test/Element.test.html.js

@@ -123,6 +123,74 @@ describe('Element', () => {
       expect($el.getAttribute('foo')).to.equal('bar');
     });
 
+    it('setAttribute should trigger attributeChangedCallback', () => {
+      const {tagName, CustomElementClass} = getObservedAttributesTestElement([
+        'foo',
+        'bar',
+      ]);
+      customElements.define(tagName, CustomElementClass);
+
+      const $el = getHTML(`<${tagName} bar="value 1"></${tagName}>`);
+      expect($el.changedAttributes[0]).to.deep.equal({
+        name: 'bar',
+        oldValue: null,
+        newValue: 'value 1',
+      });
+
+      $el.setAttribute('foo', '');
+      expect($el.changedAttributes[1]).to.deep.equal({
+        name: 'foo',
+        oldValue: null,
+        newValue: '',
+      });
+
+      $el.setAttribute('foo', '');
+      expect($el.changedAttributes[2]).to.deep.equal({
+        name: 'foo',
+        oldValue: '',
+        newValue: '',
+      });
+
+      $el.setAttribute('foo', 'value 1');
+      expect($el.changedAttributes[3]).to.deep.equal({
+        name: 'foo',
+        oldValue: '',
+        newValue: 'value 1',
+      });
+
+      /* Setting an attribute just set programmatically with the same key and
+       * value should still trigger the callback. */
+      $el.setAttribute('foo', 'value 1');
+      expect($el.changedAttributes[4]).to.deep.equal({
+        name: 'foo',
+        oldValue: 'value 1',
+        newValue: 'value 1',
+      });
+
+      $el.setAttribute('foo', 'value 2');
+      expect($el.changedAttributes[5]).to.deep.equal({
+        name: 'foo',
+        oldValue: 'value 1',
+        newValue: 'value 2',
+      });
+
+      /* Setting an attribute that was already present in the HTML with the same
+       * value should still trigger the callback. */
+      $el.setAttribute('bar', 'value 1');
+      expect($el.changedAttributes[6]).to.deep.equal({
+        name: 'bar',
+        oldValue: 'value 1',
+        newValue: 'value 1',
+      });
+
+      $el.setAttribute('bar', 'value 2');
+      expect($el.changedAttributes[7]).to.deep.equal({
+        name: 'bar',
+        oldValue: 'value 1',
+        newValue: 'value 2',
+      });
+    });
+
     it('should call removeAttribute', () => {
       const {tagName, CustomElementClass} = getObservedAttributesTestElement([
         'foo',
@@ -135,6 +203,29 @@ describe('Element', () => {
       expect($el.hasAttribute('foo')).to.be.false;
     });
 
+    it('removeAttribute should trigger attributeChangedCallback', () => {
+      const {tagName, CustomElementClass} = getObservedAttributesTestElement([
+        'foo',
+      ]);
+      customElements.define(tagName, CustomElementClass);
+      const $el = getHTML(`<${tagName} foo></${tagName}>`);
+
+      $el.removeAttribute('foo');
+      expect($el.changedAttributes[1]).to.deep.equal({
+        name: 'foo',
+        oldValue: '',
+        newValue: null,
+      });
+
+      $el.setAttribute('foo', 'value');
+      $el.removeAttribute('foo');
+      expect($el.changedAttributes[3]).to.deep.equal({
+        name: 'foo',
+        oldValue: 'value',
+        newValue: null,
+      });
+    });
+
     it('should call toggleAttribute', () => {
       const {tagName, CustomElementClass} = getObservedAttributesTestElement([
         'foo',
@@ -151,5 +242,49 @@ describe('Element', () => {
 
       expect($el.hasAttribute('foo')).to.be.true;
     });
+
+    it('toggleAttribute should trigger attributeChangedCallback', () => {
+      const {tagName, CustomElementClass} = getObservedAttributesTestElement([
+        'foo',
+      ]);
+      customElements.define(tagName, CustomElementClass);
+      const $el = getHTML(`<${tagName} foo></${tagName}>`);
+
+      /* Forcefully toggling an already present attribute on shouldn't trigger
+       * a change. */
+      $el.toggleAttribute('foo', true);
+      expect($el.changedAttributes).to.have.length(1);
+
+      /* Forcefully toggling a present attribute off. */
+      $el.toggleAttribute('foo', false);
+      expect($el.changedAttributes).to.have.length(2);
+      expect($el.changedAttributes[1]).to.deep.equal({
+        name: 'foo',
+        oldValue: '',
+        newValue: null,
+      });
+
+      /* Forcefully toggling a non-present attribute off shouldn't trigger a
+       * change. */
+      $el.toggleAttribute('foo', false);
+      expect($el.changedAttributes).to.have.length(2);
+
+      /* Forcefully toggling an absent attribute off. */
+      $el.toggleAttribute('foo', true);
+      expect($el.changedAttributes).to.have.length(3);
+      expect($el.changedAttributes[2]).to.deep.equal({
+        name: 'foo',
+        oldValue: null,
+        newValue: '',
+      });
+
+      /* Non-forcefully toggling attributes off and on. */
+      $el.toggleAttribute('foo');
+      $el.toggleAttribute('foo');
+      expect($el.changedAttributes.slice(3)).to.deep.equal([
+        {name: 'foo', oldValue: '', newValue: null},
+        {name: 'foo', oldValue: null, newValue: ''},
+      ]);
+    });
   });
 });

packages/scoped-custom-element-registry/test/utils.js

@@ -43,6 +43,13 @@ export const getObservedAttributesTestElement = (
     static get observedAttributes() {
       return observedAttributeNames;
     }
+
+    /** @type {{ name: string, oldValue: string|null, newValue: string|null}[]} */
+    changedAttributes = [];
+
+    attributeChangedCallback(name, oldValue, newValue) {
+      this.changedAttributes.push({name, oldValue, newValue});
+    }
   },
 });