:bug: Fixed a bug where event listeners weren't always removed properly.

6 files changed, 280 insertions, 86 deletions

diff --git a/examples/events/gleam.toml b/examples/events/gleam.toml
new file mode 100644
index 0000000..46a962c
--- /dev/null
+++ b/examples/events/gleam.toml
@@ -0,0 +1,7 @@
+name = "events"
+version = "1.0.0"
+target = "javascript"
+
+[dependencies]
+gleam_stdlib = "~> 0.34"
+lustre = { path = "../../" }
\ No newline at end of file
diff --git a/examples/events/manifest.toml b/examples/events/manifest.toml
new file mode 100644
index 0000000..715dadc
--- /dev/null
+++ b/examples/events/manifest.toml
@@ -0,0 +1,11 @@
+# This file was generated by Gleam
+# You typically do not need to edit this file
+
+packages = [
+  { name = "gleam_stdlib", version = "0.34.0", build_tools = ["gleam"], requirements = [], otp_app = "gleam_stdlib", source = "hex", outer_checksum = "1FB8454D2991E9B4C0C804544D8A9AD0F6184725E20D63C3155F0AEB4230B016" },
+  { name = "lustre", version = "3.0.12", build_tools = ["gleam"], requirements = ["gleam_stdlib"], source = "local", path = "../.." },
+]
+
+[requirements]
+gleam_stdlib = { version = "~> 0.34" }
+lustre = { path = "../../" }
diff --git a/examples/events/src/events.gleam b/examples/events/src/events.gleam
new file mode 100644
index 0000000..7b4a4da
--- /dev/null
+++ b/examples/events/src/events.gleam
@@ -0,0 +1,70 @@
+// IMPORTS ---------------------------------------------------------------------
+
+import gleam/int
+import lustre
+import lustre/attribute
+import lustre/element.{type Element, text}
+import lustre/element/html.{button, div, p}
+import lustre/event
+
+// MAIN ------------------------------------------------------------------------
+
+pub fn main() {
+  // A `simple` lustre application doesn't produce `Effect`s. These are best to 
+  // start with if you're just getting started with lustre or you know you don't
+  // need the runtime to manage any side effects.
+  let app = lustre.simple(init, update, view)
+  let assert Ok(_) = lustre.start(app, "[data-lustre-app]", Nil)
+}
+
+// MODEL -----------------------------------------------------------------------
+
+pub type Model {
+  Model(count: Int, active: Bool)
+}
+
+pub fn init(_) -> Model {
+  Model(count: 0, active: True)
+}
+
+// UPDATE ----------------------------------------------------------------------
+
+pub opaque type Msg {
+  Incr
+  Decr
+  Reset
+  Toggle
+}
+
+pub fn update(model: Model, msg: Msg) -> Model {
+  case msg {
+    Incr -> Model(..model, count: model.count + 1)
+    Decr -> Model(..model, count: model.count - 1)
+    Reset -> Model(..model, count: 0)
+    Toggle -> Model(..model, active: !model.active)
+  }
+}
+
+// VIEW ------------------------------------------------------------------------
+
+pub fn view(model: Model) -> Element(Msg) {
+  let on_click = fn(msg) {
+    case model.active {
+      True -> event.on_click(msg)
+      False -> attribute.none()
+    }
+  }
+
+  div([], [
+    button([on_click(Incr)], [text("+")]),
+    button([on_click(Decr)], [text("-")]),
+    button([on_click(Reset)], [text("Reset")]),
+    button([event.on_click(Toggle)], [
+      case model.active {
+        True -> text("Disable")
+        False -> text("Enable")
+      },
+    ]),
+    p([], [text(int.to_string(model.count))]),
+  ])
+}
diff --git a/src/lustre/attribute.gleam b/src/lustre/attribute.gleam
index d2099c9..7653eca 100644
--- a/src/lustre/attribute.gleam
+++ b/src/lustre/attribute.gleam
@@ -39,6 +39,11 @@ pub fn on(
   Event("on" <> name, function.compose(handler, result.replace_error(_, Nil)))
 }
 
+///
+pub fn none() -> Attribute(msg) {
+  Attribute("", dynamic.from(""), as_property: False)
+}
+
 // MANIPULATIONS ---------------------------------------------------------------
 
 /// 
@@ -64,6 +69,7 @@ pub fn to_string(attr: Attribute(msg)) -> String {
 /// 
 pub fn to_string_parts(attr: Attribute(msg)) -> Result(#(String, String), Nil) {
   case attr {
+    Attribute("", _, _) -> Error(Nil)
     Attribute("dangerous-unescaped-html", _, _) -> Error(Nil)
     Attribute(name, value, as_property) -> {
       case dynamic.classify(value) {
@@ -92,6 +98,7 @@ pub fn to_string_parts(attr: Attribute(msg)) -> Result(#(String, String), Nil) {
 ///
 pub fn to_string_builder(attr: Attribute(msg)) -> StringBuilder {
   case attr {
+    Attribute("", _, _) -> string_builder.new()
     Attribute("dangerous-unescaped-html", _, _) -> string_builder.new()
     Attribute(name, value, as_property) -> {
       case dynamic.classify(value) {
diff --git a/src/lustre/element.gleam b/src/lustre/element.gleam
index 1d18825..9593970 100644
--- a/src/lustre/element.gleam
+++ b/src/lustre/element.gleam
@@ -6,26 +6,80 @@
 import gleam/list
 import gleam/string
 import gleam/string_builder.{type StringBuilder}
-import lustre/attribute.{type Attribute}
+import lustre/attribute.{type Attribute, attribute}
 
 // TYPES -----------------------------------------------------------------------
 
 /// 
 pub opaque type Element(msg) {
-  Text(String)
-  Element(String, List(Attribute(msg)), List(Element(msg)))
-  ElementNs(String, List(Attribute(msg)), List(Element(msg)), String)
+  Text(content: String)
+  Element(
+    namespace: String,
+    tag: String,
+    attrs: List(Attribute(msg)),
+    children: List(Element(msg)),
+    self_closing: Bool,
+    void: Bool,
+  )
 }
 
 // CONSTRUCTORS ----------------------------------------------------------------
 
 /// 
+/// 
+/// 🚨 Because Lustre is primarily used to create HTML, this function spcieal-cases
+///    the following tags to be self-closing:
+/// 
+///    - area
+///    - base
+///    - br
+///    - col
+///    - embed
+///    - hr
+///    - img
+///    - input
+///    - link
+///    - meta
+///    - param
+///    - source
+///    - track
+///    - wbr
+/// 
+///    This will only affect the output of `to_string` and `to_string_builder`!
+///    If you need to render any of these tags with children, *or* you want to
+///    render some other tag as self-closing or void, use [`advanced`](#advanced)
+///    instead.
+/// 
 pub fn element(
   tag: String,
   attrs: List(Attribute(msg)),
   children: List(Element(msg)),
 ) -> Element(msg) {
-  Element(tag, attrs, children)
+  case tag {
+    "area"
+    | "base"
+    | "br"
+    | "col"
+    | "embed"
+    | "hr"
+    | "img"
+    | "input"
+    | "link"
+    | "meta"
+    | "param"
+    | "source"
+    | "track"
+    | "wbr" -> Element("", tag, attrs, [], False, True)
+    _ ->
+      Element(
+        namespace: "",
+        tag: tag,
+        attrs: attrs,
+        children: children,
+        self_closing: False,
+        void: False,
+      )
+  }
 }
 
 ///
@@ -35,7 +89,26 @@ pub fn namespaced(
   attrs: List(Attribute(msg)),
   children: List(Element(msg)),
 ) -> Element(msg) {
-  ElementNs(tag, attrs, children, namespace)
+  Element(
+    namespace: namespace,
+    tag: tag,
+    attrs: attrs,
+    children: children,
+    self_closing: False,
+    void: False,
+  )
+}
+
+///
+pub fn advanced(
+  namespace: String,
+  tag: String,
+  attrs: List(Attribute(msg)),
+  children: List(Element(msg)),
+  self_closing: Bool,
+  void: Bool,
+) -> Element(msg) {
+  Element(tag, namespace, attrs, children, self_closing, void)
 }
 
 /// 
@@ -43,6 +116,11 @@ pub fn text(content: String) -> Element(msg) {
   Text(content)
 }
 
+///
+pub fn none() -> Element(msg) {
+  Text("")
+}
+
 fn escape(escaped: String, content: String) -> String {
   case content {
     "<" <> rest -> escape(escaped <> "&lt;", rest)
@@ -64,18 +142,14 @@ fn escape(escaped: String, content: String) -> String {
 pub fn map(element: Element(a), f: fn(a) -> b) -> Element(b) {
   case element {
     Text(content) -> Text(content)
-    Element(tag, attrs, children) ->
+    Element(namespace, tag, attrs, children, self_closing, void) ->
       Element(
+        namespace,
         tag,
         list.map(attrs, attribute.map(_, f)),
         list.map(children, map(_, f)),
-      )
-    ElementNs(tag, attrs, children, namespace) ->
-      ElementNs(
-        tag,
-        list.map(attrs, attribute.map(_, f)),
-        list.map(children, map(_, f)),
-        namespace,
+        self_closing,
+        void,
       )
   }
 }
@@ -97,77 +171,68 @@ fn to_string_builder_helper(
   raw_text: Bool,
 ) -> StringBuilder {
   case element {
+    Text("") -> string_builder.new()
     Text(content) if raw_text -> string_builder.from_string(content)
     Text(content) -> string_builder.from_string(escape("", content))
 
-    Element("area" as tag, attrs, _)
-    | Element("base" as tag, attrs, _)
-    | Element("br" as tag, attrs, _)
-    | Element("col" as tag, attrs, _)
-    | Element("embed" as tag, attrs, _)
-    | Element("hr" as tag, attrs, _)
-    | Element("img" as tag, attrs, _)
-    | Element("input" as tag, attrs, _)
-    | Element("link" as tag, attrs, _)
-    | Element("meta" as tag, attrs, _)
-    | Element("param" as tag, attrs, _)
-    | Element("source" as tag, attrs, _)
-    | Element("track" as tag, attrs, _)
-    | Element("wbr" as tag, attrs, _) -> {
+    Element(namespace, tag, attrs, _, self_closing, _) if self_closing -> {
       let html = string_builder.from_string("<" <> tag)
-      let #(attrs, _) = attrs_to_string_builder(attrs)
+      let #(attrs, _) =
+        attrs_to_string_builder(case namespace {
+          "" -> attrs
+          _ -> [attribute("xmlns", namespace), ..attrs]
+        })
 
       html
       |> string_builder.append_builder(attrs)
-      |> string_builder.append(">")
+      |> string_builder.append("/>")
     }
 
-    Element("style" as tag, attrs, children)
-    | Element("script" as tag, attrs, children) -> {
+    Element(namespace, tag, attrs, _, _, void) if void -> {
       let html = string_builder.from_string("<" <> tag)
-      let #(attrs, _) = attrs_to_string_builder(attrs)
+      let #(attrs, _) =
+        attrs_to_string_builder(case namespace {
+          "" -> attrs
+          _ -> [attribute("xmlns", namespace), ..attrs]
+        })
 
       html
       |> string_builder.append_builder(attrs)
       |> string_builder.append(">")
-      |> children_to_string_builder(children, True)
-      |> string_builder.append("</" <> tag <> ">")
     }
 
-    Element(tag, attrs, children) -> {
+    // Style and script tags are special beacuse they need to contain unescape
+    // text content and not escaped HTML content.
+    Element("", "style" as tag, attrs, children, False, False)
+    | Element("", "script" as tag, attrs, children, False, False) -> {
       let html = string_builder.from_string("<" <> tag)
-      let #(attrs, inner_html) = attrs_to_string_builder(attrs)
+      let #(attrs, _) = attrs_to_string_builder(attrs)
 
-      case inner_html {
-        "" ->
-          html
-          |> string_builder.append_builder(attrs)
-          |> string_builder.append(">")
-          |> children_to_string_builder(children, raw_text)
-          |> string_builder.append("</" <> tag <> ">")
-        _ ->
-          html
-          |> string_builder.append_builder(attrs)
-          |> string_builder.append(">" <> inner_html <> "</" <> tag <> ">")
-      }
+      html
+      |> string_builder.append_builder(attrs)
+      |> string_builder.append(">")
+      |> children_to_string_builder(children, True)
+      |> string_builder.append("</" <> tag <> ">")
     }
 
-    ElementNs(tag, attrs, children, namespace) -> {
+    Element(namespace, tag, attrs, children, _, _) -> {
       let html = string_builder.from_string("<" <> tag)
-      let #(attrs, inner_html) = attrs_to_string_builder(attrs)
+      let #(attrs, inner_html) =
+        attrs_to_string_builder(case namespace {
+          "" -> attrs
+          _ -> [attribute("xmlns", namespace), ..attrs]
+        })
 
       case inner_html {
         "" ->
           html
           |> string_builder.append_builder(attrs)
-          |> string_builder.append(" xmlns=\"" <> namespace <> "\"")
           |> string_builder.append(">")
           |> children_to_string_builder(children, raw_text)
           |> string_builder.append("</" <> tag <> ">")
         _ ->
           html
           |> string_builder.append_builder(attrs)
-          |> string_builder.append(" xmlns=\"" <> namespace <> "\"")
           |> string_builder.append(">" <> inner_html <> "</" <> tag <> ">")
       }
     }
diff --git a/src/runtime.ffi.mjs b/src/runtime.ffi.mjs
index 45b05bf..97aca0a 100644
--- a/src/runtime.ffi.mjs
+++ b/src/runtime.ffi.mjs
@@ -2,26 +2,41 @@ import { Empty } from "./gleam.mjs";
 import { map as result_map } from "../gleam_stdlib/gleam/result.mjs";
 
 export function morph(prev, curr, dispatch, parent) {
-  if (curr[3]) {
-    return prev?.nodeType === 1 &&
-      prev.nodeName === curr[0].toUpperCase() &&
-      prev.namespaceURI === curr[3]
-      ? morphElement(prev, curr, curr[3], dispatch, parent)
-      : createElement(prev, curr, curr[3], dispatch, parent);
+  // The current node is an `Element` and the previous DOM node is also a DOM
+  // element.
+  if (curr?.tag && prev?.nodeType === 1) {
+    const nodeName = curr.tag.toUpperCase();
+    const ns = curr.namespace || null;
+
+    // If the current node and the existing DOM node have the same tag and
+    // namespace, we can morph them together: keeping the DOM node intact and just
+    // updating its attributes and children.
+    if (prev.nodeName === nodeName && prev.namespaceURI === ns) {
+      return morphElement(prev, curr, dispatch, parent);
+    }
+    // Otherwise, we need to replace the DOM node with a new one. The `createElement`
+    // function will handle replacing the existing DOM node for us.
+    else {
+      return createElement(prev, curr, dispatch, parent);
+    }
   }
 
-  if (curr[2]) {
-    return prev?.nodeType === 1 && prev.nodeName === curr[0].toUpperCase()
-      ? morphElement(prev, curr, null, dispatch, parent)
-      : createElement(prev, curr, null, dispatch, parent);
+  // The current node is an `Element` but the previous DOM node either did not
+  // exist or it is not a DOM element (eg it might be a text or comment node).
+  if (curr?.tag) {
+    return createElement(prev, curr, dispatch, parent);
   }
 
-  if (typeof curr?.[0] === "string") {
+  // The current node is a `Text`.
+  if (typeof curr?.content === "string") {
     return prev?.nodeType === 3
       ? morphText(prev, curr)
       : createText(prev, curr);
   }
 
+  // If someone was naughty and tried to pass in something other than a Lustre
+  // element (or if there is an actual bug with the runtime!) we'll render a
+  // comment and ask them to report the issue.
   return document.createComment(
     [
       "[internal lustre error] I couldn't work out how to render this element. This",
@@ -34,14 +49,16 @@ export function morph(prev, curr, dispatch, parent) {
 
 // ELEMENTS --------------------------------------------------------------------
 
-function createElement(prev, curr, ns, dispatch, parent = null) {
-  const el = ns
-    ? document.createElementNS(ns, curr[0])
-    : document.createElement(curr[0]);
+function createElement(prev, curr, dispatch, parent = null) {
+  const el = curr.namespace
+    ? document.createElementNS(curr.namespace, curr.tag)
+    : document.createElement(curr.tag);
 
-  el.$lustre = {};
+  el.$lustre = {
+    __registered_events: new Set(),
+  };
 
-  let attr = curr[1];
+  let attr = curr.attrs;
   let dangerousUnescapedHtml = "";
 
   while (attr.head) {
@@ -51,16 +68,16 @@ function createElement(prev, curr, ns, dispatch, parent = null) {
       morphAttr(el, attr.head[0], `${el.style.cssText} ${attr.head[1]}`);
     } else if (attr.head[0] === "dangerous-unescaped-html") {
       dangerousUnescapedHtml += attr.head[1];
-    } else {
+    } else if (attr.head[0] !== "") {
       morphAttr(el, attr.head[0], attr.head[1], dispatch);
     }
 
     attr = attr.tail;
   }
 
-  if (customElements.get(curr[0])) {
-    el._slot = curr[2];
-  } else if (curr[0] === "slot") {
+  if (customElements.get(curr.tag)) {
+    el._slot = curr.children;
+  } else if (curr.tag === "slot") {
     let child = new Empty();
     let parentWithSlot = parent;
 
@@ -80,7 +97,7 @@ function createElement(prev, curr, ns, dispatch, parent = null) {
   } else if (dangerousUnescapedHtml) {
     el.innerHTML = dangerousUnescapedHtml;
   } else {
-    let child = curr[2];
+    let child = curr.children;
     while (child.head) {
       el.appendChild(morph(null, child.head, dispatch, el));
       child = child.tail;
@@ -92,15 +109,17 @@ function createElement(prev, curr, ns, dispatch, parent = null) {
   return el;
 }
 
-function morphElement(prev, curr, ns, dispatch, parent) {
-  const prevAttrs = prev.attributes;
+function morphElement(prev, curr, dispatch, parent) {
+  const prevAttrs = Object.fromEntries(prev.attributes);
   const currAttrs = new Map();
 
   // This can happen if we're morphing an existing DOM element that *wasn't*
   // initially created by lustre.
-  prev.$lustre ??= {};
+  prev.$lustre ??= { __registered_events: new Set() };
 
-  let currAttr = curr[1];
+  // We're going to convert the Gleam List of attributes into a JavaScript Map
+  // so its easier to lookup specific attributes.
+  let currAttr = curr.attrs;
   while (currAttr.head) {
     if (currAttr.head[0] === "class" && currAttrs.has("class")) {
       currAttrs.set(
@@ -120,13 +139,15 @@ function morphElement(prev, curr, ns, dispatch, parent) {
         currAttr.head[0],
         `${currAttrs.get("dangerous-unescaped-html")} ${currAttr.head[1]}`
       );
-    } else {
+    } else if (currAttr.head[0] !== "") {
       currAttrs.set(currAttr.head[0], currAttr.head[1]);
     }
 
     currAttr = currAttr.tail;
   }
 
+  // TODO: Event listeners aren't currently removed when they are removed from
+  // the attributes list. This is a bug!
   for (const { name, value: prevValue } of prevAttrs) {
     if (!currAttrs.has(name)) {
       prev.removeAttribute(name);
@@ -140,13 +161,25 @@ function morphElement(prev, curr, ns, dispatch, parent) {
     }
   }
 
+  for (const name of prev.$lustre.__registered_events) {
+    if (!currAttrs.has(name)) {
+      const event = name.slice(2).toLowerCase();
+
+      prev.removeEventListener(event, prev.$lustre[`${name}Handler`]);
+      prev.$lustre.__registered_events.delete(name);
+
+      delete prev.$lustre[name];
+      delete prev.$lustre[`${name}Handler`];
+    }
+  }
+
   for (const [name, value] of currAttrs) {
     morphAttr(prev, name, value, dispatch);
   }
 
-  if (customElements.get(curr[0])) {
-    prev._slot = curr[2];
-  } else if (curr[0] === "slot") {
+  if (customElements.get(curr.tag)) {
+    prev._slot = curr.children;
+  } else if (curr.tag === "slot") {
     let prevChild = prev.firstChild;
     let currChild = new Empty();
     let parentWithSlot = parent;
@@ -177,7 +210,7 @@ function morphElement(prev, curr, ns, dispatch, parent) {
     prev.innerHTML = currAttrs.get("dangerous-unescaped-html");
   } else {
     let prevChild = prev.firstChild;
-    let currChild = curr[2];
+    let currChild = curr.children;
 
     while (prevChild) {
       if (currChild.head) {
@@ -227,6 +260,7 @@ function morphAttr(el, name, value, dispatch) {
 
       el.$lustre[name] = value;
       el.$lustre[`${name}Handler`] = handler;
+      el.$lustre.__registered_events.add(name);
 
       break;
     }
@@ -239,7 +273,7 @@ function morphAttr(el, name, value, dispatch) {
 // TEXT ------------------------------------------------------------------------
 
 function createText(prev, curr) {
-  const el = document.createTextNode(curr[0]);
+  const el = document.createTextNode(curr.content);
 
   if (prev) prev.replaceWith(el);
   return el;
@@ -247,7 +281,7 @@ function createText(prev, curr) {
 
 function morphText(prev, curr) {
   const prevValue = prev.nodeValue;
-  const currValue = curr[0];
+  const currValue = curr.text;
 
   if (!currValue) {
     prev?.remove();