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.

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;
 }
 

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] || [];
 }

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");
+    });
+  }
+);

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
 

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)