From d4d939bd6627c6bf0d06ad5bfffa589240cfcf31 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Tue, 26 Nov 2024 18:54:26 +0000
Subject: [PATCH] PERF: Remove dynamic `<template>` invocations (#29942)

Using Ember's `<template>` dynamically is not supported. For every invocation, glimmer-vm has to run one-time setup, and will cache the result indefinitely. This leads to significant memory leaks, and eventual OOM errors.

This commit updates a handful of cases. We'll be following up with the more complex ones, and a linting rule to avoid re-introducing the problem in future.
---
 .../discourse/app/components/d-button.gjs     | 11 +++-----
 .../app/form-kit/components/fk/field.gjs      | 26 +++++++++++--------
 .../discourse/app/helpers/plugin-outlet.gjs   | 12 +++++----
 .../raw-views/topic-bulk-select-dropdown.gjs  | 20 +++++++-------
 .../components/topic-notifications-button.gjs | 11 +++++---
 5 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/d-button.gjs b/app/assets/javascripts/discourse/app/components/d-button.gjs
index f5756a71d35..dcfd972ef9e 100644
--- a/app/assets/javascripts/discourse/app/components/d-button.gjs
+++ b/app/assets/javascripts/discourse/app/components/d-button.gjs
@@ -7,6 +7,7 @@ import { htmlSafe } from "@ember/template";
 import { or } from "truth-helpers";
 import GlimmerComponentWithDeprecatedParentView from "discourse/components/glimmer-component-with-deprecated-parent-view";
 import concatClass from "discourse/helpers/concat-class";
+import element from "discourse/helpers/element";
 import icon from "discourse-common/helpers/d-icon";
 import deprecated from "discourse-common/lib/deprecated";
 import { i18n } from "discourse-i18n";
@@ -161,18 +162,14 @@ export default class DButton extends GlimmerComponentWithDeprecatedParentView {
   }
 
   get wrapperElement() {
-    const { href, type } = this.args;
-
-    return href
-      ? <template><a href={{href}} ...attributes>{{yield}}</a></template>
-      : <template>
-          <button type={{or type "button"}} ...attributes>{{yield}}</button>
-        </template>;
+    return element(this.args.href ? "a" : "button");
   }
 
   <template>
     {{! template-lint-disable no-pointer-down-event-binding }}
     <this.wrapperElement
+      href={{@href}}
+      type={{unless @href (or @type "button")}}
       {{! For legacy compatibility. Prefer passing class as attributes. }}
       class={{concatClass
         @class
diff --git a/app/assets/javascripts/discourse/app/form-kit/components/fk/field.gjs b/app/assets/javascripts/discourse/app/form-kit/components/fk/field.gjs
index 073f5b87d77..d1cd7eb9f1d 100644
--- a/app/assets/javascripts/discourse/app/form-kit/components/fk/field.gjs
+++ b/app/assets/javascripts/discourse/app/form-kit/components/fk/field.gjs
@@ -18,6 +18,19 @@ import FKControlToggle from "discourse/form-kit/components/fk/control/toggle";
 import FKControlWrapper from "discourse/form-kit/components/fk/control-wrapper";
 import FKRow from "discourse/form-kit/components/fk/row";
 
+const RowColWrapper = <template>
+  <FKRow as |row|>
+    <row.Col @size={{@size}}>
+      {{yield}}
+    </row.Col>
+  </FKRow>
+</template>;
+
+const EmptyWrapper = <template>
+  {{! template-lint-disable no-yield-only }}
+  {{yield}}
+</template>;
+
 export default class FKField extends Component {
   @tracked field;
   @tracked name;
@@ -72,18 +85,9 @@ export default class FKField extends Component {
 
   get wrapper() {
     if (this.args.size) {
-      return <template>
-        <FKRow as |row|>
-          <row.Col @size={{@size}}>
-            {{yield}}
-          </row.Col>
-        </FKRow>
-      </template>;
+      return RowColWrapper;
     } else {
-      return <template>
-        {{! template-lint-disable no-yield-only }}
-        {{yield}}
-      </template>;
+      return EmptyWrapper;
     }
   }
 
diff --git a/app/assets/javascripts/discourse/app/helpers/plugin-outlet.gjs b/app/assets/javascripts/discourse/app/helpers/plugin-outlet.gjs
index f695a967ab2..ba0305b674f 100644
--- a/app/assets/javascripts/discourse/app/helpers/plugin-outlet.gjs
+++ b/app/assets/javascripts/discourse/app/helpers/plugin-outlet.gjs
@@ -4,6 +4,12 @@ import { connectorsExist } from "discourse/lib/plugin-connectors";
 import rawRenderGlimmer from "discourse/lib/raw-render-glimmer";
 import RawHandlebars from "discourse-common/lib/raw-handlebars";
 
+const GlimmerPluginOutletWrapper = <template>
+  {{~! no whitespace ~}}
+  <PluginOutlet @name={{@data.name}} @outletArgs={{@data.outletArgs}} />
+  {{~! no whitespace ~}}
+</template>;
+
 RawHandlebars.registerHelper("plugin-outlet", function (options) {
   const { name, tagName, outletArgs } = options.hash;
 
@@ -15,11 +21,7 @@ RawHandlebars.registerHelper("plugin-outlet", function (options) {
     rawRenderGlimmer(
       this,
       `${tagName || "span"}.hbr-ember-outlet`,
-      <template>
-        {{~! no whitespace ~}}
-        <PluginOutlet @name={{@data.name}} @outletArgs={{@data.outletArgs}} />
-        {{~! no whitespace ~}}
-      </template>,
+      GlimmerPluginOutletWrapper,
       { name, outletArgs }
     )
   );
diff --git a/app/assets/javascripts/discourse/app/raw-views/topic-bulk-select-dropdown.gjs b/app/assets/javascripts/discourse/app/raw-views/topic-bulk-select-dropdown.gjs
index 0ac85b60db3..3e1db81f25e 100644
--- a/app/assets/javascripts/discourse/app/raw-views/topic-bulk-select-dropdown.gjs
+++ b/app/assets/javascripts/discourse/app/raw-views/topic-bulk-select-dropdown.gjs
@@ -4,6 +4,16 @@ import BulkSelectTopicsDropdown from "discourse/components/bulk-select-topics-dr
 import rawRenderGlimmer from "discourse/lib/raw-render-glimmer";
 import { i18n } from "discourse-i18n";
 
+const BulkSelectGlimmerWrapper = <template>
+  <span class="bulk-select-topic-dropdown__count">
+    {{i18n "topics.bulk.selected_count" count=@data.selectedCount}}
+  </span>
+  <BulkSelectTopicsDropdown
+    @bulkSelectHelper={{@data.bulkSelectHelper}}
+    @afterBulkActionComplete={{@data.afterBulkAction}}
+  />
+</template>;
+
 export default class extends EmberObject {
   @service router;
 
@@ -20,15 +30,7 @@ export default class extends EmberObject {
     return rawRenderGlimmer(
       this,
       "div.bulk-select-topics-dropdown",
-      <template>
-        <span class="bulk-select-topic-dropdown__count">
-          {{i18n "topics.bulk.selected_count" count=@data.selectedCount}}
-        </span>
-        <BulkSelectTopicsDropdown
-          @bulkSelectHelper={{@data.bulkSelectHelper}}
-          @afterBulkActionComplete={{@data.afterBulkAction}}
-        />
-      </template>,
+      BulkSelectGlimmerWrapper,
       {
         bulkSelectHelper: this.bulkSelectHelper,
         selectedCount: this.selectedCount,
diff --git a/app/assets/javascripts/select-kit/addon/components/topic-notifications-button.gjs b/app/assets/javascripts/select-kit/addon/components/topic-notifications-button.gjs
index 3618346afc1..3395dda47b6 100644
--- a/app/assets/javascripts/select-kit/addon/components/topic-notifications-button.gjs
+++ b/app/assets/javascripts/select-kit/addon/components/topic-notifications-button.gjs
@@ -10,6 +10,11 @@ import getURL from "discourse-common/lib/get-url";
 import I18n, { i18n } from "discourse-i18n";
 import TopicNotificationsOptions from "select-kit/components/topic-notifications-options";
 
+const ParagraphWrapper = <template><p class="reason">{{yield}}</p></template>;
+const EmptyWrapper = <template>
+  {{! template-lint-disable no-yield-only}}{{yield}}
+</template>;
+
 export default class TopicNotificationsButton extends Component {
   @service currentUser;
 
@@ -86,11 +91,9 @@ export default class TopicNotificationsButton extends Component {
 
   get conditionalWrapper() {
     if (this.args.expanded) {
-      return <template><p class="reason">{{yield}}</p></template>;
+      return ParagraphWrapper;
     } else {
-      return <template>
-        {{! template-lint-disable no-yield-only}}{{yield}}
-      </template>;
+      return EmptyWrapper;
     }
   }