DEV: Improve discourse-common/deprecate implementation (take 2) (#19032)

- Count deprecations and print them to the console following QUnit runs
- In GitHub actions, write the same information as a job summary
- Add documentation to `discourse-common/lib/deprecated`
- Introduce `id` and `url` options to `deprecated`
- Introduce `withSilencedDeprecations` helper to allow testing deprecated code paths without making noise in the logs

This was previously reverted in 47035693b770a44b680d18b473035eee4778992b.

app/assets/javascripts/discourse-common/addon/lib/deprecated.js

@@ -1,17 +1,39 @@
-export default function deprecated(msg, opts = {}) {
+const handlers = [];
-  msg = ["Deprecation notice:", msg];
+const disabledDeprecations = new Set();
-  if (opts.since) {
+
-    msg.push(`(deprecated since Discourse ${opts.since})`);
+/**
+ * Display a deprecation warning with the provided message. The warning will be prefixed with the theme/plugin name
+ * if it can be automatically determined based on the current stack.
+ * @param {String} msg The deprecation message
+ * @param {Object} [options] Deprecation options
+ * @param {String} [options.id] A unique identifier for this deprecation. This should be namespaced by dots (e.g. discourse.my_deprecation)
+ * @param {String} [options.since] The Discourse version this deprecation was introduced in
+ * @param {String} [options.dropFrom] The Discourse version this deprecation will be dropped in. Typically one major version after `since`
+ * @param {String} [options.url] A URL which provides more detail about the deprecation
+ * @param {boolean} [options.raiseError] Raise an error when this deprecation is triggered. Defaults to `false`
+ */
+export default function deprecated(msg, options = {}) {
+  const { id, since, dropFrom, url, raiseError } = options;
+
+  if (id && disabledDeprecations.has(id)) {
+    return;
   }
-  if (opts.dropFrom) {
+
-    msg.push(`(removal in Discourse ${opts.dropFrom})`);
+  msg = ["Deprecation notice:", msg];
+  if (since) {
+    msg.push(`[deprecated since Discourse ${since}]`);
+  }
+  if (dropFrom) {
+    msg.push(`[removal in Discourse ${dropFrom}]`);
+  }
+  if (id) {
+    msg.push(`[deprecation id: ${id}]`);
+  }
+  if (url) {
+    msg.push(`[info: ${url}]`);
   }
   msg = msg.join(" ");
 
-  if (opts.raiseError) {
-    throw msg;
-  }
-
   let consolePrefix = "";
   if (window.Discourse) {
     // This module doesn't exist in pretty-text/wizard/etc.
@@ -19,5 +41,37 @@ export default function deprecated(msg, opts = {}) {
       require("discourse/lib/source-identifier").consolePrefix() || "";
   }
 
-  console.warn(consolePrefix, msg); //eslint-disable-line no-console
+  handlers.forEach((h) => h(msg, options));
+
+  if (raiseError) {
+    throw msg;
+  }
+
+  console.warn(...[consolePrefix, msg].filter(Boolean)); //eslint-disable-line no-console
+}
+
+/**
+ * Register a function which will be called whenever a deprecation is triggered
+ * @param {function} callback The callback function. Arguments will match those of `deprecated()`.
+ */
+export function registerDeprecationHandler(callback) {
+  handlers.push(callback);
+}
+
+/**
+ * Silence one or more deprecations while running `callback`
+ * @async
+ * @param {(string|string[])} deprecationIds A single id, or an array of ids, of deprecations to silence
+ * @param {function} callback The function to call while deprecations are silenced. Can be asynchronous.
+ */
+export async function withSilencedDeprecations(deprecationIds, callback) {
+  const idArray = [].concat(deprecationIds);
+  let result;
+  try {
+    idArray.forEach((id) => disabledDeprecations.add(id));
+    result = callback();
+  } finally {
+    idArray.forEach((id) => disabledDeprecations.delete(id));
+    return result;
+  }
 }

app/assets/javascripts/discourse/app/components/user-selector.js

@@ -16,8 +16,8 @@ export default TextField.extend({
   @on("init")
   deprecateComponent() {
     deprecated(
-      "`{{user-selector}}` is deprecated. Please use `{{email-group-user-chooser}}` instead.",
+      "The `<UserSelector>` component is deprecated. Please use `<EmailGroupUserChooser>` instead.",
-      { since: "2.7", dropFrom: "2.8" }
+      { since: "2.7", dropFrom: "2.8", id: "discourse.user-selector-component" }
     );
   },
 

app/assets/javascripts/discourse/testem.js

@@ -1,15 +1,21 @@
 const TapReporter = require("testem/lib/reporters/tap_reporter");
 const { shouldLoadPluginTestJs } = require("discourse/lib/plugin-js");
+const fs = require("fs");
 
 class Reporter {
   failReports = [];
+  deprecationCounts = new Map();
 
   constructor() {
     this._tapReporter = new TapReporter(...arguments);
   }
 
   reportMetadata(tag, metadata) {
-    if (tag === "summary-line") {
+    if (tag === "increment-deprecation") {
+      const id = metadata.id;
+      const currentCount = this.deprecationCounts.get(id) || 0;
+      this.deprecationCounts.set(id, currentCount + 1);
+    } else if (tag === "summary-line") {
       process.stdout.write(`\n${metadata.message}\n`);
     } else {
       this._tapReporter.reportMetadata(...arguments);
@@ -23,9 +29,46 @@ class Reporter {
     this._tapReporter.report(prefix, data);
   }
 
+  generateDeprecationTable() {
+    const maxIdLength = Math.max(
+      ...Array.from(this.deprecationCounts.keys()).map((k) => k.length)
+    );
+
+    let msg = `| ${"id".padEnd(maxIdLength)} | count |\n`;
+    msg += `| ${"".padEnd(maxIdLength, "-")} | ----- |\n`;
+
+    for (const [id, count] of this.deprecationCounts.entries()) {
+      const countString = count.toString();
+      msg += `| ${id.padEnd(maxIdLength)} | ${countString.padStart(5)} |\n`;
+    }
+
+    return msg;
+  }
+
+  reportDeprecations() {
+    let deprecationMessage = "[Deprecation Counter] ";
+    if (this.deprecationCounts.size > 0) {
+      const table = this.generateDeprecationTable();
+      deprecationMessage += `Test run completed with deprecations:\n\n${table}`;
+
+      if (process.env.GITHUB_ACTIONS && process.env.GITHUB_STEP_SUMMARY) {
+        let jobSummary = `### ⚠️ JS Deprecations\n\nTest run completed with deprecations:\n\n`;
+        jobSummary += table;
+        jobSummary += `\n\n`;
+
+        fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, jobSummary);
+      }
+    } else {
+      deprecationMessage += "No deprecations logged";
+    }
+    process.stdout.write(`\n${deprecationMessage}\n\n`);
+  }
+
   finish() {
     this._tapReporter.finish();
 
+    this.reportDeprecations();
+
     if (this.failReports.length > 0) {
       process.stdout.write("\nFailures:\n\n");
 

app/assets/javascripts/discourse/tests/helpers/deprecation-counter.js

@@ -0,0 +1,96 @@
+import { registerDeprecationHandler } from "@ember/debug";
+import { bind } from "discourse-common/utils/decorators";
+import { registerDeprecationHandler as registerDiscourseDeprecationHandler } from "discourse-common/lib/deprecated";
+
+export default class DeprecationCounter {
+  counts = new Map();
+  #configById = new Map();
+
+  constructor(config) {
+    for (const c of config) {
+      this.#configById.set(c.matchId, c.handler);
+    }
+  }
+
+  start() {
+    registerDeprecationHandler(this.handleEmberDeprecation);
+    registerDiscourseDeprecationHandler(this.handleDiscourseDeprecation);
+  }
+
+  @bind
+  handleEmberDeprecation(message, options, next) {
+    const { id } = options;
+    const matchingConfig = this.#configById.get(id);
+
+    if (matchingConfig !== "silence") {
+      this.incrementDeprecation(id);
+    }
+
+    next(message, options);
+  }
+
+  @bind
+  handleDiscourseDeprecation(message, options) {
+    let { id } = options;
+    id ||= "discourse.(unknown)";
+
+    this.incrementDeprecation(id);
+  }
+
+  incrementDeprecation(id) {
+    const existingCount = this.counts.get(id) || 0;
+    this.counts.set(id, existingCount + 1);
+    if (window.Testem) {
+      reportToTestem(id);
+    }
+  }
+
+  get hasDeprecations() {
+    return this.counts.size > 0;
+  }
+
+  generateTable() {
+    const maxIdLength = Math.max(
+      ...Array.from(this.counts.keys()).map((k) => k.length)
+    );
+
+    let msg = `| ${"id".padEnd(maxIdLength)} | count |\n`;
+    msg += `| ${"".padEnd(maxIdLength, "-")} | ----- |\n`;
+
+    for (const [id, count] of this.counts.entries()) {
+      const countString = count.toString();
+      msg += `| ${id.padEnd(maxIdLength)} | ${countString.padStart(5)} |\n`;
+    }
+
+    return msg;
+  }
+}
+
+function reportToTestem(id) {
+  window.Testem.useCustomAdapter(function (socket) {
+    socket.emit("test-metadata", "increment-deprecation", {
+      id,
+    });
+  });
+}
+
+export function setupDeprecationCounter(qunit) {
+  const config = window.deprecationWorkflow?.config?.workflow || {};
+  const deprecationCounter = new DeprecationCounter(config);
+
+  qunit.begin(() => deprecationCounter.start());
+
+  qunit.done(() => {
+    if (window.Testem) {
+      return;
+    } else if (deprecationCounter.hasDeprecations) {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `[Discourse Deprecation Counter] Test run completed with deprecations:\n\n${deprecationCounter.generateTable()}`
+      );
+    } else {
+      // eslint-disable-next-line no-console
+      console.log("[Discourse Deprecation Counter] No deprecations found");
+    }
+  });
+}

app/assets/javascripts/discourse/tests/integration/components/user-selector-test.js

@@ -3,6 +3,7 @@ import { setupRenderingTest } from "discourse/tests/helpers/component-test";
 import { render } from "@ember/test-helpers";
 import { query } from "discourse/tests/helpers/qunit-helpers";
 import { hbs } from "ember-cli-htmlbars";
+import { withSilencedDeprecations } from "discourse-common/lib/deprecated";
 
 function paste(element, text) {
   let e = new Event("paste");
@@ -16,8 +17,13 @@ module("Integration | Component | user-selector", function (hooks) {
   test("pasting a list of usernames", async function (assert) {
     this.set("usernames", "evil,trout");
 
-    await render(
+    await withSilencedDeprecations(
-      hbs`<UserSelector @usernames={{this.usernames}} class="test-selector" />`
+      "discourse.user-selector-component",
+      async () => {
+        await render(
+          hbs`<UserSelector @usernames={{this.usernames}} class="test-selector" />`
+        );
+      }
     );
 
     let element = query(".test-selector");
@@ -45,8 +51,13 @@ module("Integration | Component | user-selector", function (hooks) {
     this.set("usernames", "mark");
     this.set("excludedUsernames", ["jeff", "sam", "robin"]);
 
-    await render(
+    await withSilencedDeprecations(
-      hbs`<UserSelector @usernames={{this.usernames}} @excludedUsernames={{this.excludedUsernames}} class="test-selector" />`
+      "discourse.user-selector-component",
+      async () => {
+        await render(
+          hbs`<UserSelector @usernames={{this.usernames}} @excludedUsernames={{this.excludedUsernames}} class="test-selector" />`
+        );
+      }
     );
 
     let element = query(".test-selector");

app/assets/javascripts/discourse/tests/setup-tests.js

@@ -40,6 +40,7 @@ import { clearState as clearPresenceState } from "discourse/tests/helpers/presen
 import { addModuleExcludeMatcher } from "ember-cli-test-loader/test-support/index";
 import SiteSettingService from "discourse/services/site-settings";
 import jQuery from "jquery";
+import { setupDeprecationCounter } from "discourse/tests/helpers/deprecation-counter";
 
 const Plugin = $.fn.modal;
 const Modal = Plugin.Constructor;
@@ -199,6 +200,8 @@ function writeSummaryLine(message) {
 export default function setupTests(config) {
   disableCloaking();
 
+  setupDeprecationCounter(QUnit);
+
   QUnit.config.hidepassed = true;
 
   sinon.config = {

app/assets/javascripts/discourse/tests/unit/lib/deprecated-test.js

@@ -0,0 +1,178 @@
+import {
+  default as deprecated,
+  withSilencedDeprecations,
+} from "discourse-common/lib/deprecated";
+import DeprecationCounter from "discourse/tests/helpers/deprecation-counter";
+import { module, test } from "qunit";
+import Sinon from "sinon";
+
+module("Unit | Utility | deprecated", function (hooks) {
+  hooks.beforeEach(function () {
+    this.warnStub = Sinon.stub(console, "warn");
+    this.counterStub = Sinon.stub(
+      DeprecationCounter.prototype,
+      "incrementDeprecation"
+    );
+  });
+
+  test("works with just a message", function (assert) {
+    deprecated("My message");
+    assert.strictEqual(
+      this.warnStub.callCount,
+      1,
+      "console warn was called once"
+    );
+    assert.deepEqual(
+      this.warnStub.args[0],
+      ["Deprecation notice: My message"],
+      "console.warn is called with the correct arguments"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      1,
+      "incrementDeprecation was called once"
+    );
+    assert.deepEqual(
+      this.counterStub.args[0],
+      ["discourse.(unknown)"],
+      "incrementDeprecation is called with the correct arguments"
+    );
+  });
+
+  test("works with a message and id", function (assert) {
+    deprecated("My message", { id: "discourse.my_deprecation_id" });
+    assert.strictEqual(
+      this.warnStub.callCount,
+      1,
+      "console warn was called once"
+    );
+    assert.deepEqual(
+      this.warnStub.args[0],
+      [
+        "Deprecation notice: My message [deprecation id: discourse.my_deprecation_id]",
+      ],
+      "console.warn is called with the correct arguments"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      1,
+      "incrementDeprecation was called once"
+    );
+    assert.deepEqual(
+      this.counterStub.args[0],
+      ["discourse.my_deprecation_id"],
+      "incrementDeprecation is called with the correct arguments"
+    );
+  });
+
+  test("works with all other metadata", async function (assert) {
+    deprecated("My message", {
+      id: "discourse.my_deprecation_id",
+      dropFrom: "v100",
+      since: "v1",
+      url: "https://example.com",
+    });
+    assert.strictEqual(
+      this.warnStub.callCount,
+      1,
+      "console warn was called once"
+    );
+    assert.deepEqual(
+      this.warnStub.args[0],
+      [
+        "Deprecation notice: My message [deprecated since Discourse v1] [removal in Discourse v100] [deprecation id: discourse.my_deprecation_id] [info: https://example.com]",
+      ],
+      "console.warn is called with the correct arguments"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      1,
+      "incrementDeprecation was called once"
+    );
+    assert.deepEqual(
+      this.counterStub.args[0],
+      ["discourse.my_deprecation_id"],
+      "incrementDeprecation is called with the correct arguments"
+    );
+  });
+
+  test("works with raiseError", function (assert) {
+    assert.throws(
+      () =>
+        deprecated("My message", {
+          id: "discourse.my_deprecation_id",
+          raiseError: true,
+        }),
+      "Deprecation notice: My message [deprecation id: discourse.my_deprecation_id]"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      1,
+      "incrementDeprecation was called once"
+    );
+    assert.deepEqual(
+      this.counterStub.args[0],
+      ["discourse.my_deprecation_id"],
+      "incrementDeprecation is called with the correct arguments"
+    );
+  });
+
+  test("can silence individual deprecations in tests", function (assert) {
+    withSilencedDeprecations("discourse.one", () =>
+      deprecated("message", { id: "discourse.one" })
+    );
+    assert.strictEqual(
+      this.warnStub.callCount,
+      0,
+      "console.warn is not called"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      0,
+      "counter is not incremented"
+    );
+
+    deprecated("message", { id: "discourse.one" });
+    assert.strictEqual(
+      this.warnStub.callCount,
+      1,
+      "console.warn is called outside the silenced function"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      1,
+      "counter is incremented outside the silenced function"
+    );
+
+    withSilencedDeprecations("discourse.one", () =>
+      deprecated("message", { id: "discourse.two" })
+    );
+    assert.strictEqual(
+      this.warnStub.callCount,
+      2,
+      "console.warn is called for a different deprecation"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      2,
+      "counter is incremented for a different deprecation"
+    );
+  });
+
+  test("can silence multiple deprecations in tests", function (assert) {
+    withSilencedDeprecations(["discourse.one", "discourse.two"], () => {
+      deprecated("message", { id: "discourse.one" });
+      deprecated("message", { id: "discourse.two" });
+    });
+    assert.strictEqual(
+      this.warnStub.callCount,
+      0,
+      "console.warn is not called"
+    );
+    assert.strictEqual(
+      this.counterStub.callCount,
+      0,
+      "counter is not incremented"
+    );
+  });
+});