From 3e010bc88c84eff1cc32d69b1b6fe5fc16305d8d Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Tue, 16 Aug 2022 16:44:21 +0100
Subject: [PATCH] DEV: Ensure RenderGlimmer handles in-place component changes
 (#17946)

If a widget toggles between displaying two different RenderGlimmer instances, the Widget framework treats them as the same, and so `update()` is called rather than destroy/init. This commit detects this scenario and manually destroys/inits to ensure the correct component is being rendered.
---
 .../discourse/app/widgets/render-glimmer.js   | 10 ++++
 .../components/widgets/render-glimmer-test.js | 46 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/app/assets/javascripts/discourse/app/widgets/render-glimmer.js b/app/assets/javascripts/discourse/app/widgets/render-glimmer.js
index 3adc020614..ce96a1a585 100644
--- a/app/assets/javascripts/discourse/app/widgets/render-glimmer.js
+++ b/app/assets/javascripts/discourse/app/widgets/render-glimmer.js
@@ -70,6 +70,16 @@ export default class RenderGlimmer {
   }
 
   update(prev) {
+    if (
+      prev.template.__id !== this.template.__id ||
+      prev.tagName !== this.tagName
+    ) {
+      // Totally different component, but the widget framework guessed it was the
+      // same widget. Destroy old component and re-init the new one.
+      prev.destroy();
+      return this.init();
+    }
+
     this._componentInfo = prev._componentInfo;
     if (prev.data !== this.data) {
       this._componentInfo.data = this.data;
diff --git a/app/assets/javascripts/discourse/tests/integration/components/widgets/render-glimmer-test.js b/app/assets/javascripts/discourse/tests/integration/components/widgets/render-glimmer-test.js
index edbe15d97a..db86f08864 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/widgets/render-glimmer-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/widgets/render-glimmer-test.js
@@ -66,6 +66,41 @@ class DemoComponent extends ClassicComponent {
   }
 }
 
+class ToggleDemoWidget extends Widget {
+  static actionTriggered = false;
+  tagName = "div.my-widget";
+
+  buildKey() {
+    return "abc";
+  }
+
+  defaultState() {
+    return {
+      showOne: true,
+    };
+  }
+
+  html(attrs, state) {
+    const output = [
+      this.attach("button", {
+        label: "toggle",
+        className: "toggleButton",
+        action: "toggleComponent",
+      }),
+    ];
+    if (state.showOne) {
+      output.push(new RenderGlimmer(this, "div.glimmer-wrapper", hbs`One`, {}));
+    } else {
+      output.push(new RenderGlimmer(this, "div.glimmer-wrapper", hbs`Two`, {}));
+    }
+    return output;
+  }
+
+  toggleComponent() {
+    this.state.showOne = !this.state.showOne;
+  }
+}
+
 module("Integration | Component | Widget | render-glimmer", function (hooks) {
   setupRenderingTest(hooks);
 
@@ -73,11 +108,13 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
     DemoComponent.eventLog = [];
     DemoWidget.actionTriggered = false;
     this.registry.register("widget:demo-widget", DemoWidget);
+    this.registry.register("widget:toggle-demo-widget", ToggleDemoWidget);
     this.registry.register("component:demo-component", DemoComponent);
   });
 
   hooks.afterEach(function () {
     this.registry.unregister("widget:demo-widget");
+    this.registry.unregister("widget:toggle-demo-widget");
     this.registry.unregister("component:demo-component");
   });
 
@@ -217,4 +254,13 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
     new RenderGlimmer(this, "div", hbs`<TheCorrectCompiler />`);
     assert.true(true, "it doesn't raise an error for correct params");
   });
+
+  test("multiple adjacent components", async function (assert) {
+    await render(hbs`<MountWidget @widget="toggle-demo-widget" />`);
+    assert.strictEqual(query("div.glimmer-wrapper").innerText, "One");
+    await click(".toggleButton");
+    assert.strictEqual(query("div.glimmer-wrapper").innerText, "Two");
+    await click(".toggleButton");
+    assert.strictEqual(query("div.glimmer-wrapper").innerText, "One");
+  });
 });