[scoped-custom-element-registry] Fix attributeChangedCallback calling…

… for parser crated elements and toggleAttribute (#583)

- parser created custom elements call attributeChangedCallback for parser created attributes
- toggleAttribute called only when attribute value changes

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

@@ -23,6 +23,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - formAssociated set by first name's defining value or if
   CustomElementRegistryPolyfill.formAssociated set contains name
 
+### Fixed
+
+- parser created custom elements call attributeChangedCallback for parser
+  created attributes
+
+- toggleAttribute called only when attribute value changes
+
 ## [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.ts

@@ -416,6 +416,7 @@ const createStandInElement = (tagName: string): CustomElementConstructor => {
       this: HTMLElement,
       ...args: ParametersOf<CustomHTMLElement['connectedCallback']>
     ) {
+      ensureAttributesCustomized(this);
       const definition = definitionForElement.get(this);
       if (definition) {
         // Delegate out to user callback
@@ -514,6 +515,7 @@ const patchAttributes = (
   const setAttribute = elementClass.prototype.setAttribute;
   if (setAttribute) {
     elementClass.prototype.setAttribute = function (n: string, value: string) {
+      ensureAttributesCustomized(this);
       const name = n.toLowerCase();
       if (observedAttributes.has(name)) {
         const old = this.getAttribute(name);
@@ -527,6 +529,7 @@ const patchAttributes = (
   const removeAttribute = elementClass.prototype.removeAttribute;
   if (removeAttribute) {
     elementClass.prototype.removeAttribute = function (n: string) {
+      ensureAttributesCustomized(this);
       const name = n.toLowerCase();
       if (observedAttributes.has(name)) {
         const old = this.getAttribute(name);
@@ -543,19 +546,74 @@ const patchAttributes = (
       n: string,
       force?: boolean
     ) {
+      ensureAttributesCustomized(this);
       const name = n.toLowerCase();
       if (observedAttributes.has(name)) {
         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);
       }
     };
   }
 };
 
+// Helper to defer initial attribute processing for parser generated
+// custom elements. These elements are created without attributes
+// so attributes cannot be processed in the constructor. Instead,
+// these elements are customized at the first opportunity:
+// 1. when the element is connected
+// 2. when any attribute API is first used
+// 3. when the document becomes readyState === interactive (the parser is done)
+let elementsPendingAttributes: Set<CustomHTMLElement & HTMLElement> | undefined;
+if (document.readyState === 'loading') {
+  elementsPendingAttributes = new Set();
+  document.addEventListener(
+    'readystatechange',
+    () => {
+      elementsPendingAttributes!.forEach((instance) =>
+        customizeAttributes(instance, definitionForElement.get(instance)!)
+      );
+    },
+    {once: true}
+  );
+}
+
+const ensureAttributesCustomized = (
+  instance: CustomHTMLElement & HTMLElement
+) => {
+  if (!elementsPendingAttributes?.has(instance)) {
+    return;
+  }
+  customizeAttributes(instance, definitionForElement.get(instance)!);
+};
+
+// Approximate observedAttributes from the user class, since the stand-in element had none
+const customizeAttributes = (
+  instance: CustomHTMLElement & HTMLElement,
+  definition: CustomElementDefinition
+) => {
+  elementsPendingAttributes?.delete(instance);
+  if (!definition.attributeChangedCallback) {
+    return;
+  }
+  definition.observedAttributes.forEach((attr: string) => {
+    if (!instance.hasAttribute(attr)) {
+      return;
+    }
+    definition.attributeChangedCallback!.call(
+      instance,
+      attr,
+      null,
+      instance.getAttribute(attr)
+    );
+  });
+};
+
 // Helper to patch CE class hierarchy changing those CE classes created before applying the polyfill
 // to make them work with the new patched CustomElementsRegistry
 const patchHTMLElement = (elementClass: CustomElementConstructor): unknown => {
@@ -587,17 +645,17 @@ const customize = (
     new definition.elementClass();
   }
   if (definition.attributeChangedCallback) {
-    // Approximate observedAttributes from the user class, since the stand-in element had none
-    definition.observedAttributes.forEach((attr) => {
-      if (instance.hasAttribute(attr)) {
-        definition.attributeChangedCallback!.call(
-          instance,
-          attr,
-          null,
-          instance.getAttribute(attr)
-        );
-      }
-    });
+    // Note, these checks determine if the element is being parser created.
+    // and has no attributes when created. In this case, it may have attributes
+    // in HTML that are immediately processed. To handle this, the instance
+    // is added to a set and its attributes are customized at first
+    // opportunity (e.g. when connected or when the parser completes and the
+    // document becomes interactive).
+    if (elementsPendingAttributes !== undefined && !instance.hasAttributes()) {
+      elementsPendingAttributes.add(instance);
+    } else {
+      customizeAttributes(instance, definition);
+    }
   }
   if (isUpgrade && definition.connectedCallback && instance.isConnected) {
     definition.connectedCallback.call(instance);

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

@@ -1,6 +1,27 @@
 <html>
   <body>
     <script src="../scoped-custom-element-registry.min.js"></script>
+
+    <script>
+      // Test element for testing attribute processing of parser created
+      // elements.
+      customElements.define(
+        'parsed-el',
+        class extends HTMLElement {
+          static observedAttributes = ['a', 'b'];
+          attributeChanges = [];
+          attributeChangedCallback(name, old, value) {
+            this.attributeChanges.push({name, old, value});
+          }
+        }
+      );
+      const imp = document.createElement('parsed-el');
+      imp.setAttribute('a', 'ia');
+      imp.id = 'imperative-parsed-el';
+      document.body.append(imp);
+    </script>
+    <parsed-el id="parsed-el" a="a" b="b"></parsed-el>
+
     <script type="module">
       import {runTests} from '@web/test-runner-mocha';
 

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

@@ -119,7 +119,9 @@ describe('Element', () => {
       const $el = document.createElement(tagName);
 
       $el.setAttribute('foo', 'bar');
-
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'foo', old: null, value: 'bar'},
+      ]);
       expect($el.getAttribute('foo')).to.equal('bar');
     });
 
@@ -131,7 +133,10 @@ describe('Element', () => {
       const $el = getHTML(`<${tagName} foo></${tagName}>`);
 
       $el.removeAttribute('foo');
-
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'foo', old: null, value: ''},
+        {name: 'foo', old: '', value: null},
+      ]);
       expect($el.hasAttribute('foo')).to.be.false;
     });
 
@@ -148,8 +153,27 @@ describe('Element', () => {
 
       $el.setAttribute('foo', '');
       $el.toggleAttribute('foo', true);
-
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'foo', old: null, value: ''},
+      ]);
       expect($el.hasAttribute('foo')).to.be.true;
     });
+
+    it('should call attributeChangedCallback for parser created element', () => {
+      const $el = document.getElementById('parsed-el');
+      expect($el).to.be.ok;
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'a', old: null, value: 'a'},
+        {name: 'b', old: null, value: 'b'},
+      ]);
+    });
+
+    it('should call attributeChangedCallback for imperative created element while parsing', () => {
+      const $el = document.getElementById('imperative-parsed-el');
+      expect($el).to.be.ok;
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'a', old: null, value: 'ia'},
+      ]);
+    });
   });
 });

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

@@ -43,6 +43,10 @@ export const getObservedAttributesTestElement = (
     static get observedAttributes() {
       return observedAttributeNames;
     }
+    attributeChanges = [];
+    attributeChangedCallback(name, old, value) {
+      this.attributeChanges.push({name, old, value});
+    }
   },
 });