fix(runtime): better boolean attribute handling (#6413)

* fix(runtime): better boolean attribute handling

* chore: revert stencil weirdness

* chore: more bool attr testing

---------

Co-authored-by: John Jenkins <john.jenkins@nanoporetech.com>

Author: johnjenkins

src/runtime/proxy-component.ts

@@ -378,10 +378,16 @@ export const proxyComponent = (
             return;
           }
 
-          const propDesc = Object.getOwnPropertyDescriptor(prototype, propName);
+          // special handling of boolean attributes. Null (removal) means false.
+          // everything else means true (including an empty string
+          const propFlags = members.find(([m]) => m === propName);
+          if (propFlags && propFlags[1][0] & MEMBER_FLAGS.Boolean) {
+            (newValue as any) = newValue === null || newValue === 'false' ? false : true;
+          }
+
           // test whether this property either has no 'getter' or if it does, does it also have a 'setter'
           // before attempting to write back to component props
-          newValue = newValue === null && typeof this[propName] === 'boolean' ? (false as any) : newValue;
+          const propDesc = Object.getOwnPropertyDescriptor(prototype, propName);
           if (newValue != this[propName] && (!propDesc.get || !!propDesc.set)) {
             this[propName] = newValue;
           }

src/runtime/test/attr.spec.tsx

@@ -67,6 +67,14 @@ describe('attribute', () => {
 
       expect(root.textContent).toBe('false');
       expect(root.bool).toBe(false);
+
+      // reset
+      root.setAttribute('bool', '');
+      expect(root.bool).toBe(true);
+
+      // check setAttribute
+      root.setAttribute('bool', 'false');
+      expect(root.bool).toBe(false);
     });
 
     it('set boolean, undefined when missing attribute', async () => {
@@ -111,6 +119,14 @@ describe('attribute', () => {
 
       expect(root.textContent).toBe('true');
       expect(root.bool).toBe(true);
+
+      // reset
+      root.removeAttribute('bool');
+      expect(root.bool).toBe(false);
+
+      // check setAttribute
+      root.setAttribute('bool', 'true');
+      expect(root.bool).toBe(true);
     });
 
     it('set boolean true from no attribute value', async () => {
@@ -133,6 +149,14 @@ describe('attribute', () => {
 
       expect(root.textContent).toBe('true');
       expect(root.bool).toBe(true);
+
+      // reset
+      root.removeAttribute('bool');
+      expect(root.bool).toBe(false);
+
+      // check setAttribute
+      (root as HTMLElement).setAttribute('bool', '');
+      expect(root.bool).toBe(true);
     });
 
     it('set boolean true from empty string', async () => {
@@ -155,6 +179,44 @@ describe('attribute', () => {
 
       expect(root.textContent).toBe('true');
       expect(root.bool).toBe(true);
+
+      // reset
+      root.removeAttribute('bool');
+      expect(root.bool).toBe(false);
+
+      // check setAttribute
+      root.setAttribute('bool', '');
+      expect(root.bool).toBe(true);
+    });
+
+    it('set boolean true from any other string apart from "false"', async () => {
+      @Component({ tag: 'cmp-a' })
+      class CmpA {
+        @Prop() bool: boolean;
+        render() {
+          return `${this.bool}`;
+        }
+      }
+
+      const { root } = await newSpecPage({
+        components: [CmpA],
+        html: `<cmp-a bool="nice"></cmp-a>`,
+      });
+
+      expect(root).toEqualHtml(`
+        <cmp-a bool="nice">true</cmp-a>
+      `);
+
+      expect(root.textContent).toBe('true');
+      expect(root.bool).toBe(true);
+
+      // reset
+      root.removeAttribute('bool');
+      expect(root.bool).toBe(false);
+
+      // check setAttribute
+      root.setAttribute('bool', 'anything');
+      expect(root.bool).toBe(true);
     });
 
     it('set zero', async () => {