[FIX] blockdom: properly handle falsy attributes

This commit fixes some issues with falsy attributes not being properly
set/removed in various situations. Also, the behaviour was not
consistent between normal attribute (key/value) and generic attributes
(pair or object)

src/blockdom/attributes.ts

@@ -1,6 +1,6 @@
 import type { Setter } from "./block_compiler";
 
-const { setAttribute, removeAttribute } = Element.prototype;
+const { setAttribute: elemSetAttribute, removeAttribute } = Element.prototype;
 const tokenList = DOMTokenList.prototype;
 const tokenListAdd = tokenList.add;
 const tokenListRemove = tokenList.remove;
@@ -14,11 +14,23 @@ const wordRegexp = /\s+/;
  * file.
  */
 
+function setAttribute(this: HTMLElement, key: string, value: any) {
+  switch (value) {
+    case false:
+    case undefined:
+      removeAttribute.call(this, key);
+      break;
+    case true:
+      elemSetAttribute.call(this, key, "");
+      break;
+    default:
+      elemSetAttribute.call(this, key, value);
+  }
+}
+
 export function createAttrUpdater(attr: string): Setter<HTMLElement> {
   return function (this: HTMLElement, value: any) {
-    if (value !== false && value !== undefined) {
+    setAttribute.call(this, attr, value);
-      setAttribute.call(this, attr, value === true ? "" : value);
-    }
   };
 }
 

tests/blockdom/block_attributes.test.ts

@@ -26,6 +26,26 @@ test("simple attribute", async () => {
   expect(fixture.innerHTML).toBe(`<div hello="owl"></div>`);
 });
 
+test("updating attribute with falsy value", async () => {
+  const block = createBlock('<div block-attribute-0="hello"></div>');
+  const tree = block([false]);
+
+  mount(tree, fixture);
+  expect(fixture.innerHTML).toBe(`<div></div>`);
+
+  patch(tree, block(["owl"]));
+  expect(fixture.innerHTML).toBe(`<div hello="owl"></div>`);
+
+  patch(tree, block([false]));
+  expect(fixture.innerHTML).toBe(`<div></div>`);
+
+  patch(tree, block(["owl"]));
+  expect(fixture.innerHTML).toBe(`<div hello="owl"></div>`);
+
+  patch(tree, block([undefined]));
+  expect(fixture.innerHTML).toBe(`<div></div>`);
+});
+
 test("dynamic attribute (pair)", async () => {
   const block = createBlock('<div block-attributes="0"></div>');
   const tree = block([["hello", "world"]]);
@@ -37,6 +57,20 @@ test("dynamic attribute (pair)", async () => {
   expect(fixture.innerHTML).toBe(`<div ola="mundo"></div>`);
 });
 
+test("dynamic attribute (pair, with false value)", async () => {
+  const block = createBlock('<div block-attributes="0"></div>');
+  const tree = block([["hello", false]]);
+
+  mount(tree, fixture);
+  expect(fixture.innerHTML).toBe(`<div></div>`);
+
+  patch(tree, block([["hello", "world"]]));
+  expect(fixture.innerHTML).toBe(`<div hello="world"></div>`);
+
+  patch(tree, block([["hello", false]]));
+  expect(fixture.innerHTML).toBe(`<div></div>`);
+});
+
 test("dynamic attribute (object)", async () => {
   const block = createBlock('<div block-attributes="0"></div>');
   const tree = block([{ hello: "world" }]);
@@ -48,6 +82,23 @@ test("dynamic attribute (object)", async () => {
   expect(fixture.innerHTML).toBe(`<div ola="mundo"></div>`);
 });
 
+test("dynamic attribute (object), with falsy values", async () => {
+  const block = createBlock('<div block-attributes="0"></div>');
+  const tree = block([{ hello: "world", blip: false }]);
+
+  mount(tree, fixture);
+  expect(fixture.innerHTML).toBe(`<div hello="world"></div>`);
+
+  patch(tree, block([{ ola: "mundo", blip: undefined }]));
+  expect(fixture.innerHTML).toBe(`<div ola="mundo"></div>`);
+
+  patch(tree, block([{ ola: false, blip: 1 }]));
+  expect(fixture.innerHTML).toBe(`<div blip="1"></div>`);
+
+  patch(tree, block([{ ola: undefined, blip: undefined }]));
+  expect(fixture.innerHTML).toBe(`<div></div>`);
+});
+
 test("class attribute", async () => {
   const block = createBlock('<div block-attribute-0="class"></div>');
   const tree = block(["fire"]);