DEV: Improve args deprecation on plugin outlets (#27885)

app/assets/javascripts/discourse/app/components/header/contents.gjs

@@ -2,6 +2,7 @@ import Component from "@glimmer/component";
 import { hash } from "@ember/helper";
 import { service } from "@ember/service";
 import { and } from "truth-helpers";
+import deprecatedOutletArgument from "discourse/helpers/deprecated-outlet-argument";
 import BootstrapModeNotice from "../bootstrap-mode-notice";
 import PluginOutlet from "../plugin-outlet";
 import HomeLogo from "./home-logo";
@@ -28,10 +29,19 @@ export default class Contents extends Component {
       <PluginOutlet
         @name="header-contents__before"
         @outletArgs={{hash
-          topic=this.header.topic
           topicInfo=this.header.topicInfo
           topicInfoVisible=this.header.topicInfoVisible
         }}
+        @deprecatedArgs={{hash
+          topic=(deprecatedOutletArgument
+            value=this.header.topic
+            message="The argument 'topic' is deprecated on the outlet 'header-contents__before', use 'topicInfo' or 'topicInfoVisible' instead"
+            id="discourse.plugin-connector.deprecated-arg.header-contents.topic"
+            since="3.3.0.beta4-dev"
+            dropFrom="3.4.0"
+            silence="discourse.header-service-topic"
+          )
+        }}
       />
       {{#if this.site.desktopView}}
         {{#if @sidebarEnabled}}
@@ -67,10 +77,19 @@ export default class Contents extends Component {
         <PluginOutlet
           @name="before-header-panel"
           @outletArgs={{hash
-            topic=this.header.topic
             topicInfo=this.header.topicInfo
             topicInfoVisible=this.header.topicInfoVisible
           }}
+          @deprecatedArgs={{hash
+            topic=(deprecatedOutletArgument
+              value=this.header.topic
+              message="The argument 'topic' is deprecated on the outlet 'before-header-panel', use 'topicInfo' or 'topicInfoVisible' instead"
+              id="discourse.plugin-connector.deprecated-arg.header-contents.topic"
+              since="3.3.0.beta4-dev"
+              dropFrom="3.4.0"
+              silence="discourse.header-service-topic"
+            )
+          }}
         />
       </div>
       <div class="panel" role="navigation">{{yield}}</div>
@@ -78,19 +97,37 @@ export default class Contents extends Component {
         <PluginOutlet
           @name="after-header-panel"
           @outletArgs={{hash
-            topic=this.header.topic
             topicInfo=this.header.topicInfo
             topicInfoVisible=this.header.topicInfoVisible
           }}
+          @deprecatedArgs={{hash
+            topic=(deprecatedOutletArgument
+              value=this.header.topic
+              message="The argument 'topic' is deprecated on the outlet 'after-header-panel', use 'topicInfo' or 'topicInfoVisible' instead"
+              id="discourse.plugin-connector.deprecated-arg.header-contents.topic"
+              since="3.3.0.beta4-dev"
+              dropFrom="3.4.0"
+              silence="discourse.header-service-topic"
+            )
+          }}
         />
       </div>
       <PluginOutlet
         @name="header-contents__after"
         @outletArgs={{hash
-          topic=this.header.topic
           topicInfo=this.header.topicInfo
           topicInfoVisible=this.header.topicInfoVisible
         }}
+        @deprecatedArgs={{hash
+          topic=(deprecatedOutletArgument
+            value=this.header.topic
+            message="The argument 'topic' is deprecated on the outlet 'header-contents__after', use 'topicInfo' or 'topicInfoVisible' instead"
+            id="discourse.plugin-connector.deprecated-arg.header-contents.topic"
+            since="3.3.0.beta4-dev"
+            dropFrom="3.4.0"
+            silence="discourse.header-service-topic"
+          )
+        }}
       />
     </div>
   </template>

app/assets/javascripts/discourse/app/components/plugin-connector.js

@@ -1,7 +1,9 @@
 import Component from "@ember/component";
 import { computed, defineProperty } from "@ember/object";
-import { buildArgsWithDeprecations } from "discourse/lib/plugin-connectors";
+import {
-import deprecated from "discourse-common/lib/deprecated";
+  buildArgsWithDeprecations,
+  deprecatedArgumentValue,
+} from "discourse/lib/plugin-connectors";
 import { afterRender } from "discourse-common/utils/decorators";
 
 let _decorators = {};
@@ -30,19 +32,23 @@ export default Component.extend({
     });
 
     const deprecatedArgs = this.deprecatedArgs || {};
+    const connectorInfo = {
+      outletName: this.connector?.outletName,
+      connectorName: this.connector?.connectorName,
+      classModuleName: this.connector?.classModuleName,
+      templateModule: this.connector?.templateModule,
+      layoutName: this.layoutName,
+    };
+
     Object.keys(deprecatedArgs).forEach((key) => {
       defineProperty(
         this,
         key,
         computed("deprecatedArgs", () => {
-          deprecated(
+          return deprecatedArgumentValue(deprecatedArgs[key], {
-            `The ${key} property is deprecated, but is being used in ${this.layoutName}`,
+            ...connectorInfo,
-            {
+            argumentName: key,
-              id: "discourse.plugin-connector.deprecated-arg",
+          });
-            }
-          );
-
-          return (this.deprecatedArgs || {})[key];
         })
       );
     });
@@ -56,7 +62,11 @@ export default Component.extend({
       }
     }
 
-    const merged = buildArgsWithDeprecations(args, deprecatedArgs);
+    const merged = buildArgsWithDeprecations(
+      args,
+      deprecatedArgs,
+      connectorInfo
+    );
     connectorClass?.setupComponent?.call(this, merged, this);
   },
 

app/assets/javascripts/discourse/app/components/plugin-outlet.js

@@ -127,7 +127,8 @@ export default class PluginOutletComponent extends GlimmerComponentWithDeprecate
 
     return buildArgsWithDeprecations(
       this.outletArgs,
-      this.args.deprecatedArgs || {}
+      this.args.deprecatedArgs || {},
+      { outletName: this.args.name }
     );
   }
 

app/assets/javascripts/discourse/app/helpers/deprecated-outlet-argument.js

@@ -0,0 +1,39 @@
+export default function deprecatedOutletArgument(options) {
+  return new DeprecatedOutletArgument(options);
+}
+
+export function isDeprecatedOutletArgument(value) {
+  return value instanceof DeprecatedOutletArgument;
+}
+
+class DeprecatedOutletArgument {
+  #message;
+  #silence;
+  #valueRef;
+
+  constructor(options) {
+    this.#message = options.message;
+    this.#valueRef = () => options.value;
+    this.#silence = options.silence;
+
+    this.options = {
+      id: options.id || "discourse.plugin-connector.deprecated-arg",
+      since: options.since,
+      dropFrom: options.dropFrom,
+      url: options.url,
+      raiseError: options.raiseError,
+    };
+  }
+
+  get message() {
+    return this.#message;
+  }
+
+  get silence() {
+    return this.#silence;
+  }
+
+  get value() {
+    return this.#valueRef();
+  }
+}

app/assets/javascripts/discourse/app/lib/plugin-connectors.js

@@ -4,7 +4,10 @@ import {
   setComponentTemplate,
 } from "@glimmer/manager";
 import templateOnly from "@ember/component/template-only";
-import deprecated from "discourse-common/lib/deprecated";
+import { isDeprecatedOutletArgument } from "discourse/helpers/deprecated-outlet-argument";
+import deprecated, {
+  withSilencedDeprecations,
+} from "discourse-common/lib/deprecated";
 import { buildRawConnectorCache } from "discourse-common/lib/raw-templates";
 
 let _connectorCache;
@@ -235,24 +238,61 @@ export function rawConnectorsFor(outletName) {
   return _rawConnectorCache[outletName] || [];
 }
 
-export function buildArgsWithDeprecations(args, deprecatedArgs) {
+export function buildArgsWithDeprecations(args, deprecatedArgs, opts = {}) {
   const output = {};
 
   Object.keys(args).forEach((key) => {
     Object.defineProperty(output, key, { value: args[key] });
   });
 
-  Object.keys(deprecatedArgs).forEach((key) => {
+  Object.keys(deprecatedArgs).forEach((argumentName) => {
-    Object.defineProperty(output, key, {
+    Object.defineProperty(output, argumentName, {
       get() {
-        deprecated(`${key} is deprecated`, {
+        const deprecatedArg = deprecatedArgs[argumentName];
-          id: "discourse.plugin-connector.deprecated-arg",
-        });
 
-        return deprecatedArgs[key];
+        return deprecatedArgumentValue(deprecatedArg, {
+          ...opts,
+          argumentName,
+        });
       },
     });
   });
 
   return output;
 }
+
+export function deprecatedArgumentValue(deprecatedArg, options) {
+  if (!isDeprecatedOutletArgument(deprecatedArg)) {
+    throw new Error(
+      "deprecated argument is not defined properly, use helper `deprecatedOutletArgument` from discourse/helpers/deprecated-outlet-argument"
+    );
+  }
+
+  let message = deprecatedArg.message;
+  if (!message) {
+    if (options.outletName) {
+      message = `outlet arg \`${options.argumentName}\` is deprecated on the outlet \`${options.outletName}\``;
+    } else {
+      message = `${options.argumentName} is deprecated`;
+    }
+  }
+
+  const connectorModule =
+    options.classModuleName || options.templateModule || options.connectorName;
+
+  if (connectorModule) {
+    message += ` [used on connector ${connectorModule}]`;
+  } else if (options.layoutName) {
+    message += ` [used on ${options.layoutName}]`;
+  }
+
+  if (!deprecatedArg.silence) {
+    deprecated(message, deprecatedArg.options);
+    return deprecatedArg.value;
+  }
+
+  return withSilencedDeprecations(deprecatedArg.silence, () => {
+    deprecated(message, deprecatedArg.options);
+    return deprecatedArg.value;
+  });
+}

app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-test.gjs

@@ -7,6 +7,7 @@ import hbs from "htmlbars-inline-precompile";
 import { module, test } from "qunit";
 import sinon from "sinon";
 import PluginOutlet from "discourse/components/plugin-outlet";
+import deprecatedOutletArgument from "discourse/helpers/deprecated-outlet-argument";
 import {
   extraConnectorClass,
   extraConnectorComponent,
@@ -14,10 +15,14 @@ import {
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
 import { query } from "discourse/tests/helpers/qunit-helpers";
 import { registerTemporaryModule } from "discourse/tests/helpers/temporary-module-helper";
-import {
+import deprecated, {
   withSilencedDeprecations,
   withSilencedDeprecationsAsync,
 } from "discourse-common/lib/deprecated";
+import {
+  disableRaiseOnDeprecation,
+  enableRaiseOnDeprecation,
+} from "../../helpers/raise-on-deprecation";
 
 const TEMPLATE_PREFIX = "discourse/plugins/some-plugin/templates/connectors";
 const CLASS_PREFIX = "discourse/plugins/some-plugin/connectors";
@@ -423,6 +428,172 @@ module("Integration | Component | plugin-outlet", function (hooks) {
       "other outlet is left untouched"
     );
   });
+
+  module("deprecated arguments", function (innerHooks) {
+    innerHooks.beforeEach(function () {
+      this.consoleWarnStub = sinon.stub(console, "warn");
+      disableRaiseOnDeprecation();
+    });
+
+    innerHooks.afterEach(function () {
+      this.consoleWarnStub.restore();
+      enableRaiseOnDeprecation();
+    });
+
+    test("deprecated parameters with default message", async function (assert) {
+      await render(<template>
+        <PluginOutlet
+          @name="test-name"
+          @deprecatedArgs={{hash
+            shouldDisplay=(deprecatedOutletArgument value=true)
+          }}
+        />
+      </template>);
+
+      // deprecated argument still works
+      assert.dom(".conditional-render").exists("renders conditional outlet");
+
+      assert.strictEqual(
+        this.consoleWarnStub.callCount,
+        1,
+        "console warn was called once"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          "Deprecation notice: outlet arg `shouldDisplay` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+        ),
+        true,
+        "logs the default message to the console"
+      );
+    });
+
+    test("deprecated parameters with custom deprecation data", async function (assert) {
+      await render(<template>
+        <PluginOutlet
+          @name="test-name"
+          @deprecatedArgs={{hash
+            shouldDisplay=(deprecatedOutletArgument
+              value=true
+              message="The 'shouldDisplay' is deprecated on this test"
+              id="discourse.plugin-connector.deprecated-arg.test"
+              since="3.3.0.beta4-dev"
+              dropFrom="3.4.0"
+            )
+          }}
+        />
+      </template>);
+
+      // deprecated argument still works
+      assert.dom(".conditional-render").exists("renders conditional outlet");
+
+      assert.strictEqual(
+        this.consoleWarnStub.callCount,
+        1,
+        "console warn was called once"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          sinon.match(/The 'shouldDisplay' is deprecated on this test/)
+        ),
+        true,
+        "logs the custom deprecation message to the console"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          sinon.match(
+            /deprecation id: discourse.plugin-connector.deprecated-arg.test/
+          )
+        ),
+        true,
+        "logs custom deprecation id"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          sinon.match(/deprecated since Discourse 3.3.0.beta4-dev/)
+        ),
+        true,
+        "logs deprecation since information"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          sinon.match(/removal in Discourse 3.4.0/)
+        ),
+        true,
+        "logs dropFrom information"
+      );
+    });
+
+    test("silence nested deprecations", async function (assert) {
+      const deprecatedData = {
+        get display() {
+          deprecated("Test message", {
+            id: "discourse.deprecation-that-should-not-be-logged",
+          });
+          return true;
+        },
+      };
+
+      await render(<template>
+        <PluginOutlet
+          @name="test-name"
+          @deprecatedArgs={{hash
+            shouldDisplay=(deprecatedOutletArgument
+              value=deprecatedData.display
+              silence="discourse.deprecation-that-should-not-be-logged"
+            )
+          }}
+        />
+      </template>);
+
+      // deprecated argument still works
+      assert.dom(".conditional-render").exists("renders conditional outlet");
+
+      assert.strictEqual(
+        this.consoleWarnStub.callCount,
+        1,
+        "console warn was called once"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          sinon.match(
+            /deprecation id: discourse.deprecation-that-should-not-be-logged/
+          )
+        ),
+        false,
+        "does not log silence deprecation"
+      );
+      assert.strictEqual(
+        this.consoleWarnStub.calledWith(
+          sinon.match(
+            /deprecation id: discourse.plugin-connector.deprecated-arg/
+          )
+        ),
+        true,
+        "logs expected deprecation"
+      );
+    });
+
+    test("unused arguments", async function (assert) {
+      await render(<template>
+        <PluginOutlet
+          @name="test-name"
+          @outletArgs={{hash shouldDisplay=true}}
+          @deprecatedArgs={{hash
+            argNotUsed=(deprecatedOutletArgument value=true)
+          }}
+        />
+      </template>);
+
+      // deprecated argument still works
+      assert.dom(".conditional-render").exists("renders conditional outlet");
+
+      assert.strictEqual(
+        this.consoleWarnStub.callCount,
+        0,
+        "console warn not called"
+      );
+    });
+  });
 });
 
 module(
@@ -434,10 +605,8 @@ module(
       registerTemporaryModule(
         `${TEMPLATE_PREFIX}/test-name/my-connector`,
         hbs`
-          <span class="outletArgHelloValue">{{@outletArgs.hello}}</span><span
+          <span class="outletArgHelloValue">{{@outletArgs.hello}}</span>
-            class="thisHelloValue"
+          <span class="thisHelloValue">{{this.hello}}</span>`
-          >{{this.hello}}</span>
-        `
       );
     });
 
@@ -583,6 +752,205 @@ module(
 
       assert.dom(".outletArgHelloValue").doesNotExist();
     });
+
+    module("deprecated arguments", function (innerHooks) {
+      innerHooks.beforeEach(function () {
+        this.consoleWarnStub = sinon.stub(console, "warn");
+        disableRaiseOnDeprecation();
+      });
+
+      innerHooks.afterEach(function () {
+        this.consoleWarnStub.restore();
+        enableRaiseOnDeprecation();
+      });
+
+      test("using classic PluginConnector by default", async function (assert) {
+        await render(hbs`
+        <PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
+      `);
+
+        // deprecated argument still works
+        assert.dom(".outletArgHelloValue").hasText("world");
+        assert.dom(".thisHelloValue").hasText("world");
+
+        assert.strictEqual(
+          this.consoleWarnStub.callCount,
+          2,
+          "console warn was called twice"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for @outletArgs.hello"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [used on connector discourse/plugins/some-plugin/templates/connectors/test-name/my-connector] [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for this.hello"
+        );
+      });
+
+      test("using templateOnly by default when @defaultGlimmer=true", async function (assert) {
+        await render(hbs`
+        <PluginOutlet
+          @name="test-name"
+          @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}}
+          @defaultGlimmer={{true}}
+        />
+      `);
+
+        // deprecated argument still works
+        assert.dom(".outletArgHelloValue").hasText("world");
+        assert.dom(".thisHelloValue").hasText(""); // `this.` unavailable in templateOnly components
+
+        assert.strictEqual(
+          this.consoleWarnStub.callCount,
+          1,
+          "console warn was called once"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for @outletArgs.hello"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [used on connector discourse/plugins/some-plugin/templates/connectors/test-name/my-connector] [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          false,
+          "does not log the message for this.hello"
+        );
+      });
+
+      test("using simple object when provided", async function (assert) {
+        registerTemporaryModule(`${CLASS_PREFIX}/test-name/my-connector`, {
+          setupComponent(args, component) {
+            component.reopen({
+              get hello() {
+                return args.hello + " from setupComponent";
+              },
+            });
+          },
+        });
+
+        await render(hbs`
+        <PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
+      `);
+
+        // deprecated argument still works
+        assert.dom(".outletArgHelloValue").hasText("world");
+        assert.dom(".thisHelloValue").hasText("world from setupComponent");
+
+        assert.strictEqual(
+          this.consoleWarnStub.callCount,
+          2,
+          "console warn was called twice"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for @outletArgs.hello"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [used on connector discourse/plugins/some-plugin/connectors/test-name/my-connector] [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for this.hello"
+        );
+      });
+
+      test("using custom component class if provided", async function (assert) {
+        registerTemporaryModule(
+          `${CLASS_PREFIX}/test-name/my-connector`,
+          class MyOutlet extends Component {
+            get hello() {
+              return this.args.outletArgs.hello + " from custom component";
+            }
+          }
+        );
+
+        await render(hbs`
+        <PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
+      `);
+
+        // deprecated argument still works
+        assert.dom(".outletArgHelloValue").hasText("world");
+        assert.dom(".thisHelloValue").hasText("world from custom component");
+
+        assert.strictEqual(
+          this.consoleWarnStub.callCount,
+          2,
+          "console warn was called twice"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for @outletArgs.hello"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for this.hello"
+        );
+      });
+
+      test("using custom templateOnly() if provided", async function (assert) {
+        registerTemporaryModule(
+          `${CLASS_PREFIX}/test-name/my-connector`,
+          templateOnly()
+        );
+
+        await render(hbs`
+        <PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
+      `);
+
+        // deprecated argument still works
+        assert.dom(".outletArgHelloValue").hasText("world");
+        assert.dom(".thisHelloValue").hasText(""); // `this.` unavailable in templateOnly components
+
+        assert.strictEqual(
+          this.consoleWarnStub.callCount,
+          1,
+          "console warn was called twice"
+        );
+        assert.strictEqual(
+          this.consoleWarnStub.calledWith(
+            "Deprecation notice: outlet arg `hello` is deprecated on the outlet `test-name` [deprecation id: discourse.plugin-connector.deprecated-arg]"
+          ),
+          true,
+          "logs the expected message for @outletArgs.hello"
+        );
+      });
+
+      test("unused arguments", async function (assert) {
+        await render(hbs`
+          <PluginOutlet @name="test-name" @outletArgs={{hash hello="world"}} @deprecatedArgs={{hash argNotUsed=(deprecated-outlet-argument value="not used")}} />
+        `);
+
+        // deprecated argument still works
+        assert.dom(".outletArgHelloValue").hasText("world");
+        assert.dom(".thisHelloValue").hasText("world");
+
+        assert.strictEqual(
+          this.consoleWarnStub.callCount,
+          0,
+          "console warn was called twice"
+        );
+      });
+    });
   }
 );