From 249900f25635ac03f3191cadc41d8c0e51d4fad1 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 13 Sep 2024 17:51:44 +0100
Subject: [PATCH] DEV: Track RenderGlimmer data on a per-key basis

Previously we were tracking the entire `@data` object. That means a change to any argument would cause everything to be invalidated. This commit changes it so that we track on a per-key basis, so that we reduce unnecessary re-rendering.
---
 .../discourse/app/widgets/render-glimmer.js   | 24 ++++++++++---
 ...limmer-test.js => render-glimmer-test.gjs} | 36 ++++++++++++++++---
 2 files changed, 51 insertions(+), 9 deletions(-)
 rename app/assets/javascripts/discourse/tests/integration/components/widgets/{render-glimmer-test.js => render-glimmer-test.gjs} (93%)

diff --git a/app/assets/javascripts/discourse/app/widgets/render-glimmer.js b/app/assets/javascripts/discourse/app/widgets/render-glimmer.js
index c9b57d62de4..5874cf43678 100644
--- a/app/assets/javascripts/discourse/app/widgets/render-glimmer.js
+++ b/app/assets/javascripts/discourse/app/widgets/render-glimmer.js
@@ -1,8 +1,8 @@
 import { hasInternalComponentManager } from "@glimmer/manager";
-import { tracked } from "@glimmer/tracking";
 import { setComponentTemplate } from "@ember/component";
 import templateOnly from "@ember/component/template-only";
 import { assert } from "@ember/debug";
+import { TrackedObject } from "@ember-compat/tracked-built-ins";
 import { createWidgetFrom } from "discourse/widgets/widget";
 
 const INITIAL_CLASSES = Symbol("RENDER_GLIMMER_INITIAL_CLASSES");
@@ -139,8 +139,22 @@ export default class RenderGlimmer {
     }
 
     this._componentInfo = prev._componentInfo;
-    if (prev.data !== this.data) {
-      this._componentInfo.data = this.data;
+
+    const data = this._componentInfo.data;
+    const incomingData = this.data;
+
+    if (incomingData) {
+      const keysToRemove = new Set(Object.keys(data));
+
+      for (let [key, value] of Object.entries(incomingData)) {
+        if (data[key] !== value) {
+          data[key] = value;
+        }
+        keysToRemove.delete(key);
+      }
+      for (const key of keysToRemove) {
+        delete data[key];
+      }
     }
 
     return null;
@@ -161,7 +175,7 @@ export default class RenderGlimmer {
     this._componentInfo = new ComponentInfo({
       element,
       component,
-      data: this.data,
+      data: new TrackedObject(this.data),
       setWrapperElementAttrs: (attrs) =>
         this.updateElementAttrs(element, attrs),
     });
@@ -231,7 +245,7 @@ export function registerWidgetShim(name, tagName, template) {
 }
 
 class ComponentInfo {
-  @tracked data;
+  data;
   element;
   component;
   setWrapperElementAttrs;
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.gjs
similarity index 93%
rename from app/assets/javascripts/discourse/tests/integration/components/widgets/render-glimmer-test.js
rename to app/assets/javascripts/discourse/tests/integration/components/widgets/render-glimmer-test.gjs
index 1c8de7153b5..997ab6e90ee 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.gjs
@@ -3,6 +3,7 @@ import templateOnly from "@ember/component/template-only";
 import { click, fillIn, render } from "@ember/test-helpers";
 import { hbs } from "ember-cli-htmlbars";
 import { module, test } from "qunit";
+import DButton from "discourse/components/d-button";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
 import { exists, query } from "discourse/tests/helpers/qunit-helpers";
 import widgetHbs from "discourse/widgets/hbs-compiler";
@@ -61,7 +62,6 @@ class DemoWidget extends Widget {
 class DemoComponent extends ClassicComponent {
   static eventLog = [];
   classNames = ["demo-component"];
-  layout = hbs`<DButton class="component-action-button" @label="component_action" @action={{@action}} /><p class='action-state'>{{@widgetActionTriggered}}</p>`;
 
   init() {
     DemoComponent.eventLog.push("init");
@@ -87,6 +87,22 @@ class DemoComponent extends ClassicComponent {
     super.willDestroy(...arguments);
     DemoComponent.eventLog.push("willDestroy");
   }
+
+  @bind
+  logEvent(msg) {
+    DemoComponent.eventLog.push(msg);
+  }
+
+  <template>
+    <DButton
+      class="component-action-button"
+      @label="component_action"
+      @action={{@action}}
+    />
+    <p class="action-state">{{@widgetActionTriggered}}</p>
+    {{this.logEvent "arg1 rendered" @arg1}}
+    {{this.logEvent "dynamicArg rendered" @dynamicArg}}
+  </template>
 }
 
 class ToggleDemoWidget extends Widget {
@@ -207,7 +223,13 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
 
     assert.deepEqual(
       DemoComponent.eventLog,
-      ["init", "didReceiveAttrs", "didInsertElement"],
+      [
+        "init",
+        "didReceiveAttrs",
+        "arg1 rendered",
+        "dynamicArg rendered",
+        "didInsertElement",
+      ],
       "component is initialized correctly"
     );
 
@@ -217,7 +239,7 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
     await click(".my-widget button");
     assert.deepEqual(
       DemoComponent.eventLog,
-      ["didReceiveAttrs", "didReceiveAttrs"], // once for input, once for event
+      ["didReceiveAttrs", "dynamicArg rendered", "didReceiveAttrs"], // once for input, once for event
       "component is notified of attr change during widget rerender"
     );
 
@@ -235,7 +257,13 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
     await fillIn("input.dynamic-value-input", "visibleAgain");
     assert.deepEqual(
       DemoComponent.eventLog,
-      ["init", "didReceiveAttrs", "didInsertElement"],
+      [
+        "init",
+        "didReceiveAttrs",
+        "arg1 rendered",
+        "dynamicArg rendered",
+        "didInsertElement",
+      ],
       "component can be reinitialized"
     );
   });