diff --git a/app/assets/javascripts/discourse/app/components/plugin-connector.js b/app/assets/javascripts/discourse/app/components/plugin-connector.js index 42f829efbfc..5ae2459f6dd 100644 --- a/app/assets/javascripts/discourse/app/components/plugin-connector.js +++ b/app/assets/javascripts/discourse/app/components/plugin-connector.js @@ -20,9 +20,6 @@ export default Component.extend({ init() { this._super(...arguments); - const connector = this.connector; - this.set("layoutName", connector.templateName); - const args = this.args || {}; Object.keys(args).forEach((key) => { defineProperty( @@ -50,15 +47,17 @@ export default Component.extend({ ); }); - const connectorClass = this.get("connector.connectorClass"); - this.set("actions", connectorClass.actions); + const connectorClass = this.connector.connectorClass; + this.set("actions", connectorClass?.actions); - for (const [name, action] of Object.entries(this.actions)) { - this.set(name, action.bind(this)); + if (this.actions) { + for (const [name, action] of Object.entries(this.actions)) { + this.set(name, action.bind(this)); + } } const merged = buildArgsWithDeprecations(args, deprecatedArgs); - connectorClass.setupComponent.call(this, merged, this); + connectorClass?.setupComponent?.call(this, merged, this); }, didReceiveAttrs() { @@ -77,13 +76,13 @@ export default Component.extend({ willDestroyElement() { this._super(...arguments); - const connectorClass = this.get("connector.connectorClass"); - connectorClass.teardownComponent.call(this, this); + const connectorClass = this.connector.connectorClass; + connectorClass?.teardownComponent?.call(this, this); }, send(name, ...args) { - const connectorClass = this.get("connector.connectorClass"); - const action = connectorClass.actions[name]; + const connectorClass = this.connector.connectorClass; + const action = connectorClass?.actions?.[name]; return action ? action.call(this, ...args) : this._super(name, ...args); }, }); diff --git a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js index 0a9e07146c8..1315bbcc1b2 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js @@ -1,6 +1,12 @@ 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, + setComponentTemplate, +} from "@glimmer/manager"; +import templateOnly from "@ember/component/template-only"; let _connectorCache; let _rawConnectorCache; @@ -18,13 +24,6 @@ export function extraConnectorClass(name, obj) { _extraConnectorClasses[name] = obj; } -const DefaultConnectorClass = { - actions: {}, - shouldRender: () => true, - setupComponent() {}, - teardownComponent() {}, -}; - function findOutlets(keys, callback) { keys.forEach(function (res) { const segments = res.split("/"); @@ -58,9 +57,20 @@ function findClass(outletName, uniqueName) { const id = `${outletName}/${uniqueName}`; let foundClass = _extraConnectorClasses[id] || _classPaths[id]; - return foundClass - ? Object.assign({}, DefaultConnectorClass, foundClass) - : DefaultConnectorClass; + return foundClass; +} + +/** + * Sets component template, ignoring errors if it's already set to the same template + */ +function safeSetComponentTemplate(template, component) { + try { + setComponentTemplate(template, component); + } catch (e) { + if (getComponentTemplate(component) !== template) { + throw e; + } + } } /** @@ -71,21 +81,77 @@ export function expireConnectorCache() { _connectorCache = null; } +class ConnectorInfo { + #componentClass; + #templateOnly; + + constructor(outletName, connectorName, connectorClass, template) { + this.outletName = outletName; + this.connectorName = connectorName; + this.connectorClass = connectorClass; + this.template = template; + } + + get componentClass() { + return (this.#componentClass ??= this.#buildComponentClass()); + } + + get templateOnly() { + return (this.#templateOnly ??= this.#buildTemplateOnlyClass()); + } + + get classicClassNames() { + return `${this.outletName}-outlet ${this.connectorName}`; + } + + #buildComponentClass() { + const klass = this.connectorClass; + if (klass && hasInternalComponentManager(klass)) { + safeSetComponentTemplate(this.template, klass); + this.#warnUnusableHooks(); + return klass; + } else { + return false; + } + } + + #buildTemplateOnlyClass() { + const component = templateOnly(); + setComponentTemplate(this.template, component); + this.#warnUnusableHooks(); + return component; + } + + #warnUnusableHooks() { + for (const methodName of [ + "actions", + "setupComponent", + "teardownComponent", + ]) { + if (this.connectorClass?.[methodName]) { + deprecated( + `actions, setupComponent and teardownComponent hooks cannot be used with Glimmer plugin outlets. Define a component class instead. (${this.outletName}/${this.connectorName}).`, + { id: "discourse.plugin-outlet-classic-hooks" } + ); + } + } + } +} + function buildConnectorCache() { _connectorCache = {}; findOutlets( DiscourseTemplateMap.keys(), - (outletName, resource, uniqueName) => { - _connectorCache[outletName] = _connectorCache[outletName] || []; + (outletName, resource, connectorName) => { + _connectorCache[outletName] ||= []; - _connectorCache[outletName].push({ - outletName, - templateName: resource, - template: require(DiscourseTemplateMap.resolve(resource)).default, - classNames: `${outletName}-outlet ${uniqueName}`, - connectorClass: findClass(outletName, uniqueName), - }); + const template = require(DiscourseTemplateMap.resolve(resource)).default; + const connectorClass = findClass(outletName, connectorName); + + _connectorCache[outletName].push( + new ConnectorInfo(outletName, connectorName, connectorClass, template) + ); } ); } @@ -99,7 +165,8 @@ export function connectorsFor(outletName) { export function renderedConnectorsFor(outletName, args, context) { return connectorsFor(outletName).filter((con) => { - return con.connectorClass.shouldRender(args, context); + const shouldRender = con.connectorClass?.shouldRender; + return !shouldRender || shouldRender(args, context); }); } diff --git a/app/assets/javascripts/discourse/app/templates/components/plugin-outlet.hbs b/app/assets/javascripts/discourse/app/templates/components/plugin-outlet.hbs index 900b9effa43..2639a58e88a 100644 --- a/app/assets/javascripts/discourse/app/templates/components/plugin-outlet.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/plugin-outlet.hbs @@ -5,24 +5,40 @@ }} {{#each this.connectors as |c|}} - + {{#if c.componentClass}} + + {{else if @defaultGlimmer}} + + {{else}} + + {{/if}} {{/each}} {{else}} {{! The modern path: no wrapper element = no classic component }} {{#each this.connectors as |c|}} - + {{#if c.componentClass}} + + {{else if @defaultGlimmer}} + + {{else}} + + {{/if}} {{/each}} {{/if}} \ No newline at end of file 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 5d65655b8b3..a3cf9ff01b8 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 @@ -7,6 +7,9 @@ import { extraConnectorClass } from "discourse/lib/plugin-connectors"; import { hbs } from "ember-cli-htmlbars"; import { registerTemporaryModule } from "discourse/tests/helpers/temporary-module-helper"; 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"; const PREFIX = "discourse/plugins/some-plugin/templates/connectors"; @@ -147,3 +150,143 @@ module("Integration | Component | plugin-outlet", function (hooks) { ); }); }); + +module( + "Integration | Component | plugin-outlet | connector class definitions", + function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function () { + registerTemporaryModule( + `${PREFIX}/test-name/my-connector`, + hbs`{{@outletArgs.hello}}{{this.hello}}` + ); + }); + + test("uses classic PluginConnector by default", async function (assert) { + await render( + hbs`` + ); + + assert.dom(".outletArgHelloValue").hasText("world"); + assert.dom(".thisHelloValue").hasText("world"); + }); + + test("uses templateOnly by default when @defaultGlimmer=true", async function (assert) { + await render( + hbs`` + ); + + assert.dom(".outletArgHelloValue").hasText("world"); + assert.dom(".thisHelloValue").hasText(""); // `this.` unavailable in templateOnly components + }); + + test("uses simple object if provided", async function (assert) { + this.set("someBoolean", true); + + extraConnectorClass("test-name/my-connector", { + shouldRender(args) { + return args.someBoolean; + }, + + setupComponent(args, component) { + component.reopen({ + get hello() { + return args.hello + " from setupComponent"; + }, + }); + }, + }); + + await render( + hbs`` + ); + + assert.dom(".outletArgHelloValue").hasText("world"); + assert.dom(".thisHelloValue").hasText("world from setupComponent"); + + this.set("someBoolean", false); + await settled(); + + assert.dom(".outletArgHelloValue").doesNotExist(); + }); + + test("ignores classic hooks for glimmer components", async function (assert) { + extraConnectorClass("test-name/my-connector", { + setupComponent(args, component) { + component.reopen({ + get hello() { + return args.hello + " from setupComponent"; + }, + }); + }, + }); + + await withSilencedDeprecationsAsync( + "discourse.plugin-outlet-classic-hooks", + async () => { + await render( + hbs`` + ); + } + ); + + assert.dom(".outletArgHelloValue").hasText("world"); + assert.dom(".thisHelloValue").hasText(""); + }); + + test("uses custom component class if provided", async function (assert) { + this.set("someBoolean", true); + + extraConnectorClass( + "test-name/my-connector", + class MyOutlet extends Component { + static shouldRender(args) { + return args.someBoolean; + } + + get hello() { + return this.args.outletArgs.hello + " from custom component"; + } + } + ); + + await render( + hbs`` + ); + + assert.dom(".outletArgHelloValue").hasText("world"); + assert.dom(".thisHelloValue").hasText("world from custom component"); + + this.set("someBoolean", false); + await settled(); + + assert.dom(".outletArgHelloValue").doesNotExist(); + }); + + test("uses custom templateOnly() if provided", async function (assert) { + this.set("someBoolean", true); + + extraConnectorClass( + "test-name/my-connector", + Object.assign(templateOnly(), { + shouldRender(args) { + return args.someBoolean; + }, + }) + ); + + await render( + hbs`` + ); + + assert.dom(".outletArgHelloValue").hasText("world"); + assert.dom(".thisHelloValue").hasText(""); // `this.` unavailable in templateOnly components + + this.set("someBoolean", false); + await settled(); + + assert.dom(".outletArgHelloValue").doesNotExist(); + }); + } +);