From 16c6ab8661b38051b14d68297afdc8504bc1175b Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 18 Aug 2023 12:07:10 +0100
Subject: [PATCH] DEV: Allow plugin outlets to be defined using gjs (#23142)

Previously we were discovering plugin outlets by checking first for dedicated template files, and then looking for classes to match them. This doesn't work for components which are entirely defined in JS (e.g. those authored with gjs, or those which are re-exports of a colocated component).

This commit refactors our detection logic to look for both class and template modules in a single pass. It also refactors things so that the modules themselves are required lazily when needd, rather than all being loaded during app boot.
---
 .../addon/lib/raw-templates.js                | 13 ++-
 .../discourse/app/lib/plugin-connectors.js    | 93 ++++++++++---------
 .../components/plugin-outlet-test.js          | 36 ++++++-
 app/models/theme.rb                           |  2 +-
 app/models/theme_field.rb                     |  5 +-
 5 files changed, 95 insertions(+), 54 deletions(-)

diff --git a/app/assets/javascripts/discourse-common/addon/lib/raw-templates.js b/app/assets/javascripts/discourse-common/addon/lib/raw-templates.js
index 5a8bd75e100..2a7c2a3a747 100644
--- a/app/assets/javascripts/discourse-common/addon/lib/raw-templates.js
+++ b/app/assets/javascripts/discourse-common/addon/lib/raw-templates.js
@@ -31,17 +31,20 @@ export function findRawTemplate(name) {
   );
 }
 
-export function buildRawConnectorCache(findOutlets) {
+export function buildRawConnectorCache() {
   let result = {};
-  findOutlets(
-    Object.keys(__DISCOURSE_RAW_TEMPLATES),
-    (outletName, resource) => {
+  Object.keys(__DISCOURSE_RAW_TEMPLATES).forEach((resource) => {
+    const segments = resource.split("/");
+    const connectorIndex = segments.indexOf("connectors");
+
+    if (connectorIndex >= 0) {
+      const outletName = segments[connectorIndex + 1];
       result[outletName] ??= [];
       result[outletName].push({
         template: __DISCOURSE_RAW_TEMPLATES[resource],
       });
     }
-  );
+  });
   return result;
 }
 
diff --git a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js
index 1315bbcc1b2..2a7e1411f8a 100644
--- a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js
+++ b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js
@@ -1,6 +1,5 @@
 import { buildRawConnectorCache } from "discourse-common/lib/raw-templates";
 import deprecated from "discourse-common/lib/deprecated";
-import DiscourseTemplateMap from "discourse-common/lib/discourse-template-map";
 import {
   getComponentTemplate,
   hasInternalComponentManager,
@@ -11,11 +10,9 @@ import templateOnly from "@ember/component/template-only";
 let _connectorCache;
 let _rawConnectorCache;
 let _extraConnectorClasses = {};
-let _classPaths;
 
 export function resetExtraClasses() {
   _extraConnectorClasses = {};
-  _classPaths = undefined;
 }
 
 // Note: In plugins, define a class by path and it will be wired up automatically
@@ -24,14 +21,19 @@ export function extraConnectorClass(name, obj) {
   _extraConnectorClasses[name] = obj;
 }
 
-function findOutlets(keys, callback) {
-  keys.forEach(function (res) {
-    const segments = res.split("/");
-    if (segments.includes("connectors")) {
-      const outletName = segments[segments.length - 2];
-      const uniqueName = segments[segments.length - 1];
+const OUTLET_REGEX =
+  /^discourse(\/[^\/]+)*?(?<template>\/templates)?\/connectors\/(?<outlet>[^\/]+)\/(?<name>[^\/\.]+)$/;
 
-      callback(outletName, res, uniqueName);
+function findOutlets(keys, callback) {
+  return keys.forEach((res) => {
+    const match = res.match(OUTLET_REGEX);
+    if (match) {
+      callback({
+        outletName: match.groups.outlet,
+        connectorName: match.groups.name,
+        moduleName: res,
+        isTemplate: !!match.groups.template,
+      });
     }
   });
 }
@@ -41,25 +43,6 @@ export function clearCache() {
   _rawConnectorCache = null;
 }
 
-function findClass(outletName, uniqueName) {
-  if (!_classPaths) {
-    _classPaths = {};
-    findOutlets(Object.keys(require._eak_seen), (outlet, res, un) => {
-      const possibleConnectorClass = requirejs(res).default;
-      if (possibleConnectorClass.__id) {
-        // This is the template, not the connector class
-        return;
-      }
-      _classPaths[`${outlet}/${un}`] = possibleConnectorClass;
-    });
-  }
-
-  const id = `${outletName}/${uniqueName}`;
-  let foundClass = _extraConnectorClasses[id] || _classPaths[id];
-
-  return foundClass;
-}
-
 /**
  * Sets component template, ignoring errors if it's already set to the same template
  */
@@ -85,11 +68,9 @@ class ConnectorInfo {
   #componentClass;
   #templateOnly;
 
-  constructor(outletName, connectorName, connectorClass, template) {
+  constructor(outletName, connectorName) {
     this.outletName = outletName;
     this.connectorName = connectorName;
-    this.connectorClass = connectorClass;
-    this.template = template;
   }
 
   get componentClass() {
@@ -104,10 +85,26 @@ class ConnectorInfo {
     return `${this.outletName}-outlet ${this.connectorName}`;
   }
 
+  get connectorClass() {
+    if (this.classModule) {
+      return require(this.classModule).default;
+    } else {
+      return _extraConnectorClasses[`${this.outletName}/${this.connectorName}`];
+    }
+  }
+
+  get template() {
+    if (this.templateModule) {
+      return require(this.templateModule).default;
+    }
+  }
+
   #buildComponentClass() {
     const klass = this.connectorClass;
     if (klass && hasInternalComponentManager(klass)) {
-      safeSetComponentTemplate(this.template, klass);
+      if (this.template) {
+        safeSetComponentTemplate(this.template, klass);
+      }
       this.#warnUnusableHooks();
       return klass;
     } else {
@@ -141,19 +138,31 @@ class ConnectorInfo {
 function buildConnectorCache() {
   _connectorCache = {};
 
+  const outletsByModuleName = {};
   findOutlets(
-    DiscourseTemplateMap.keys(),
-    (outletName, resource, connectorName) => {
-      _connectorCache[outletName] ||= [];
+    Object.keys(require.entries),
+    ({ outletName, connectorName, moduleName, isTemplate }) => {
+      let key = isTemplate
+        ? moduleName.replace("/templates/", "/")
+        : moduleName;
 
-      const template = require(DiscourseTemplateMap.resolve(resource)).default;
-      const connectorClass = findClass(outletName, connectorName);
+      let info = (outletsByModuleName[key] ??= new ConnectorInfo(
+        outletName,
+        connectorName
+      ));
 
-      _connectorCache[outletName].push(
-        new ConnectorInfo(outletName, connectorName, connectorClass, template)
-      );
+      if (isTemplate) {
+        info.templateModule = moduleName;
+      } else {
+        info.classModule = moduleName;
+      }
     }
   );
+
+  for (const info of Object.values(outletsByModuleName)) {
+    _connectorCache[info.outletName] ??= [];
+    _connectorCache[info.outletName].push(info);
+  }
 }
 
 export function connectorsFor(outletName) {
@@ -172,7 +181,7 @@ export function renderedConnectorsFor(outletName, args, context) {
 
 export function rawConnectorsFor(outletName) {
   if (!_rawConnectorCache) {
-    _rawConnectorCache = buildRawConnectorCache(findOutlets);
+    _rawConnectorCache = buildRawConnectorCache();
   }
   return _rawConnectorCache[outletName] || [];
 }
diff --git a/app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-test.js b/app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-test.js
index 4d44b2c426d..d880d2ad88a 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-test.js
@@ -10,8 +10,10 @@ import { getOwner } from "discourse-common/lib/get-owner";
 import Component from "@glimmer/component";
 import templateOnly from "@ember/component/template-only";
 import { withSilencedDeprecationsAsync } from "discourse-common/lib/deprecated";
+import { setComponentTemplate } from "@glimmer/manager";
 
-const PREFIX = "discourse/plugins/some-plugin/templates/connectors";
+const TEMPLATE_PREFIX = "discourse/plugins/some-plugin/templates/connectors";
+const CLASS_PREFIX = "discourse/plugins/some-plugin/connectors";
 
 module("Integration | Component | plugin-outlet", function (hooks) {
   setupRenderingTest(hooks);
@@ -52,19 +54,19 @@ module("Integration | Component | plugin-outlet", function (hooks) {
     });
 
     registerTemporaryModule(
-      `${PREFIX}/test-name/hello`,
+      `${TEMPLATE_PREFIX}/test-name/hello`,
       hbs`<span class='hello-username'>{{this.username}}</span>
         <button class='say-hello' {{on "click" (action "sayHello")}}></button>
         <button class='say-hello-using-this' {{on "click" this.sayHello}}></button>
         <span class='hello-result'>{{this.hello}}</span>`
     );
     registerTemporaryModule(
-      `${PREFIX}/test-name/hi`,
+      `${TEMPLATE_PREFIX}/test-name/hi`,
       hbs`<button class='say-hi' {{on "click" (action "sayHi")}}></button>
         <span class='hi-result'>{{this.hi}}</span>`
     );
     registerTemporaryModule(
-      `${PREFIX}/test-name/conditional-render`,
+      `${TEMPLATE_PREFIX}/test-name/conditional-render`,
       hbs`<span class="conditional-render">I only render sometimes</span>`
     );
   });
@@ -158,7 +160,7 @@ module(
 
     hooks.beforeEach(function () {
       registerTemporaryModule(
-        `${PREFIX}/test-name/my-connector`,
+        `${TEMPLATE_PREFIX}/test-name/my-connector`,
         hbs`<span class='outletArgHelloValue'>{{@outletArgs.hello}}</span><span class='thisHelloValue'>{{this.hello}}</span>`
       );
     });
@@ -290,3 +292,27 @@ module(
     });
   }
 );
+
+module(
+  "Integration | Component | plugin-outlet | gjs class definitions",
+  function (hooks) {
+    setupRenderingTest(hooks);
+
+    hooks.beforeEach(function () {
+      const template = hbs`<span class='gjs-test'>Hello world</span>`;
+      const component = templateOnly();
+      setComponentTemplate(template, component);
+
+      registerTemporaryModule(
+        `${CLASS_PREFIX}/test-name/my-connector`,
+        component
+      );
+    });
+
+    test("detects a gjs connector with no associated template file", async function (assert) {
+      await render(hbs`<PluginOutlet @name="test-name" />`);
+
+      assert.dom(".gjs-test").hasText("Hello world");
+    });
+  }
+);
diff --git a/app/models/theme.rb b/app/models/theme.rb
index de338ea363b..6d6db833b00 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -6,7 +6,7 @@ require "json_schemer"
 class Theme < ActiveRecord::Base
   include GlobalPath
 
-  BASE_COMPILER_VERSION = 71
+  BASE_COMPILER_VERSION = 72
 
   attr_accessor :child_components
 
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index b63a796c522..1fff6f85f77 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -108,7 +108,10 @@ class ThemeField < ActiveRecord::Base
           if is_raw
             js_compiler.append_raw_template(name, hbs_template)
           else
-            js_compiler.append_ember_template("discourse/templates/#{name}", hbs_template)
+            js_compiler.append_ember_template(
+              "discourse/templates/#{name.delete_prefix("/")}",
+              hbs_template,
+            )
           end
         rescue ThemeJavascriptCompiler::CompileError => ex
           js_compiler.append_js_error("discourse/templates/#{name}", ex.message)