diff --git a/app/assets/javascripts/discourse/app/components/plugin-outlet.hbs b/app/assets/javascripts/discourse/app/components/plugin-outlet.hbs index acf2e087f05..852facad01f 100644 --- a/app/assets/javascripts/discourse/app/components/plugin-outlet.hbs +++ b/app/assets/javascripts/discourse/app/components/plugin-outlet.hbs @@ -4,7 +4,7 @@ But for now, this classic component wrapper takes care of the tagName. }} <this.wrapperComponent @tagName={{@tagName}}> - {{#each this.connectors as |c|}} + {{#each (this.getConnectors) as |c|}} {{#if c.componentClass}} <c.componentClass @outletArgs={{this.outletArgsWithDeprecations}} /> {{else if @defaultGlimmer}} @@ -24,11 +24,15 @@ </this.wrapperComponent> {{else if this.connectorsExist}} {{! The modern path: no wrapper element = no classic component }} - {{#each this.connectors as |c|}} + {{#each (this.getConnectors hasBlock=(has-block)) as |c|}} {{#if c.componentClass}} - <c.componentClass @outletArgs={{this.outletArgsWithDeprecations}} /> + <c.componentClass + @outletArgs={{this.outletArgsWithDeprecations}} + >{{yield}}</c.componentClass> {{else if @defaultGlimmer}} - <c.templateOnly @outletArgs={{this.outletArgsWithDeprecations}} /> + <c.templateOnly + @outletArgs={{this.outletArgsWithDeprecations}} + >{{yield}}</c.templateOnly> {{else}} <PluginConnector @connector={{c}} @@ -38,7 +42,11 @@ @class={{c.classicClassNames}} @tagName={{or @connectorTagName ""}} @layout={{c.template}} - /> + >{{yield}}</PluginConnector> {{/if}} + {{else}} + {{yield}} {{/each}} +{{else}} + {{yield}} {{/if}} \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/plugin-outlet.js b/app/assets/javascripts/discourse/app/components/plugin-outlet.js index 0fc1af240fb..48621322c81 100644 --- a/app/assets/javascripts/discourse/app/components/plugin-outlet.js +++ b/app/assets/javascripts/discourse/app/components/plugin-outlet.js @@ -10,13 +10,15 @@ import { helperContext } from "discourse-common/lib/helpers"; import deprecated from "discourse-common/lib/deprecated"; import { get } from "@ember/object"; import { cached } from "@glimmer/tracking"; +import { bind } from "discourse-common/utils/decorators"; +import { inject as service } from "@ember/service"; const PARENT_VIEW_DEPRECATION_MSG = "parentView should not be used within plugin outlets. Use the available outlet arguments, or inject a service which can provide the context you need."; const GET_DEPRECATION_MSG = "Plugin outlet context is no longer an EmberObject - using `get()` is deprecated."; const TAG_NAME_DEPRECATION_MSG = - "The `tagName` argument to PluginOutlet is deprecated. If a wrapper element is required, define it manually around the outlet call."; + "The `tagName` argument to PluginOutlet is deprecated. If a wrapper element is required, define it manually around the outlet call. Using tagName will prevent wrapper PluginOutlets from functioning correctly."; const ARGS_DEPRECATION_MSG = "PluginOutlet arguments should now be passed using `@outletArgs=` instead of `@args=`"; @@ -48,6 +50,8 @@ const ARGS_DEPRECATION_MSG = **/ export default class PluginOutletComponent extends GlimmerComponentWithDeprecatedParentView { + @service clientErrorHandler; + context = { ...helperContext(), get parentView() { @@ -79,12 +83,24 @@ export default class PluginOutletComponent extends GlimmerComponentWithDeprecate return result; } - get connectors() { - return renderedConnectorsFor( + @bind + getConnectors({ hasBlock }) { + const connectors = renderedConnectorsFor( this.args.name, this.outletArgsWithDeprecations, this.context ); + if (connectors.length > 1 && hasBlock) { + const message = `Multiple connectors were registered for the ${this.args.name} outlet. Using the first.`; + this.clientErrorHandler.displayErrorNotice(message); + // eslint-disable-next-line no-console + console.error( + message, + connectors.map((c) => c.humanReadableName) + ); + return [connectors[0]]; + } + return connectors; } get connectorsExist() { diff --git a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js index 8960ab848b7..dfa0d0e6b71 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js @@ -99,6 +99,12 @@ class ConnectorInfo { } } + get humanReadableName() { + return `${this.outletName}/${this.connectorName} (${ + this.classModule || this.templateModule + })`; + } + #buildComponentClass() { const klass = this.connectorClass; if (klass && hasInternalComponentManager(klass)) { 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 d880d2ad88a..96043470ea4 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 @@ -11,6 +11,7 @@ import Component from "@glimmer/component"; import templateOnly from "@ember/component/template-only"; import { withSilencedDeprecationsAsync } from "discourse-common/lib/deprecated"; import { setComponentTemplate } from "@glimmer/manager"; +import sinon from "sinon"; const TEMPLATE_PREFIX = "discourse/plugins/some-plugin/templates/connectors"; const CLASS_PREFIX = "discourse/plugins/some-plugin/connectors"; @@ -69,6 +70,15 @@ module("Integration | Component | plugin-outlet", function (hooks) { `${TEMPLATE_PREFIX}/test-name/conditional-render`, hbs`<span class="conditional-render">I only render sometimes</span>` ); + + registerTemporaryModule( + `${TEMPLATE_PREFIX}/outlet-with-default/my-connector`, + hbs`<span class='result'>Plugin implementation{{#if @outletArgs.yieldCore}} {{yield}}{{/if}}</span>` + ); + registerTemporaryModule( + `${TEMPLATE_PREFIX}/outlet-with-default/clashing-connector`, + hbs`This will override my-connector and raise an error` + ); }); test("Renders a template into the outlet", async function (assert) { @@ -103,6 +113,124 @@ module("Integration | Component | plugin-outlet", function (hooks) { ); }); + module( + "as a wrapper around a default core implementation", + function (innerHooks) { + innerHooks.beforeEach(function () { + this.consoleErrorStub = sinon.stub(console, "error"); + + this.set("shouldDisplay", false); + this.set("yieldCore", false); + this.set("enableClashingConnector", false); + + extraConnectorClass("outlet-with-default/my-connector", { + shouldRender(args) { + return args.shouldDisplay; + }, + }); + + extraConnectorClass("outlet-with-default/clashing-connector", { + shouldRender(args) { + return args.enableClashingConnector; + }, + }); + + this.template = hbs` + <PluginOutlet @name="outlet-with-default" @outletArgs={{hash shouldDisplay=this.shouldDisplay yieldCore=this.yieldCore enableClashingConnector=this.enableClashingConnector}}> + <span class='result'>Core implementation</span> + </PluginOutlet> + `; + }); + + test("Can act as a wrapper around core implementation", async function (assert) { + await render(this.template); + + assert.dom(".result").hasText("Core implementation"); + + this.set("shouldDisplay", true); + await settled(); + + assert.dom(".result").hasText("Plugin implementation"); + + this.set("yieldCore", true); + await settled(); + + assert + .dom(".result") + .hasText("Plugin implementation Core implementation"); + + assert.strictEqual( + this.consoleErrorStub.callCount, + 0, + "no errors in console" + ); + }); + + test("clashing connectors for regular users", async function (assert) { + await render(this.template); + + this.set("shouldDisplay", true); + this.set("enableClashingConnector", true); + await settled(); + + assert.strictEqual( + this.consoleErrorStub.callCount, + 1, + "clash error reported to console" + ); + + assert.true( + this.consoleErrorStub + .getCall(0) + .args[0].includes("Multiple connectors"), + "console error includes message about multiple connectors" + ); + + assert + .dom(".broken-theme-alert-banner") + .doesNotExist("Banner is not shown to regular users"); + }); + + test("clashing connectors for admins", async function (assert) { + this.set("currentUser.admin", true); + await render(this.template); + + this.set("shouldDisplay", true); + this.set("enableClashingConnector", true); + await settled(); + + assert.strictEqual( + this.consoleErrorStub.callCount, + 1, + "clash error reported to console" + ); + + assert.true( + this.consoleErrorStub + .getCall(0) + .args[0].includes("Multiple connectors"), + "console error includes message about multiple connectors" + ); + + assert + .dom(".broken-theme-alert-banner") + .exists("Error banner is shown to admins"); + }); + } + ); + + test("Renders wrapped implementation if no connectors are registered", async function (assert) { + await render( + hbs` + <PluginOutlet @name="outlet-with-no-registrations"> + <span class='result'>Core implementation</span> + </PluginOutlet> + ` + ); + + assert.dom(".result").hasText("Core implementation"); + }); + test("Reevaluates shouldRender for argument changes", async function (assert) { this.set("shouldDisplay", false); await render(