From 9fbd471e89accfa4a84f03e91f68951d2a892437 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 4 Mar 2025 17:22:04 +0000 Subject: [PATCH] DEV: Improve template-override deprecations (stable) These changes are backported from `main`, so that stable users receive better feedback about this upcoming deprecation. - Update Meta topic URLs - Add theme/plugin identification to deprecation - Enable admin warning banner https://meta.discourse.org/t/355668 --- .../component-templates.js | 34 +++++++++++++-- .../discourse/app/lib/deprecated.js | 5 ++- .../services/deprecation-warning-handler.js | 6 ++- .../integration/component-templates-test.gjs | 43 ------------------- 4 files changed, 38 insertions(+), 50 deletions(-) diff --git a/app/assets/javascripts/discourse/app/instance-initializers/component-templates.js b/app/assets/javascripts/discourse/app/instance-initializers/component-templates.js index 7eb5c54c90e7b..280a677b71374 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/component-templates.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/component-templates.js @@ -4,6 +4,7 @@ import deprecated from "discourse/lib/deprecated"; import DiscourseTemplateMap from "discourse/lib/discourse-template-map"; import { isTesting } from "discourse/lib/environment"; import { RAW_TOPIC_LIST_DEPRECATION_OPTIONS } from "discourse/lib/plugin-api"; +import { getThemeInfo } from "discourse/lib/source-identifier"; let THROW_GJS_ERROR = isTesting(); @@ -18,6 +19,24 @@ export function overrideThrowGjsError(value) { const LEGACY_TOPIC_LIST_OVERRIDES = ["topic-list", "topic-list-item"]; +function sourceForModuleName(name) { + const pluginMatch = name.match(/^discourse\/plugins\/([^\/]+)\//)?.[1]; + if (pluginMatch) { + return { + type: "plugin", + name: pluginMatch, + }; + } + + const themeMatch = name.match(/^discourse\/theme-(\d+)\//)?.[1]; + if (themeMatch) { + return { + ...getThemeInfo(parseInt(themeMatch, 10)), + type: "theme", + }; + } +} + export default { after: ["populate-template-map", "mobile"], @@ -31,11 +50,15 @@ export default { } let componentName = templateKey; + const finalOverrideModuleName = moduleNames[moduleNames.length - 1]; + if (mobile) { deprecated( `Mobile-specific hbs templates are deprecated. Use responsive CSS or {{#if this.site.mobileView}} instead. [${templateKey}]`, { id: "discourse.mobile-templates", + url: "https://meta.discourse.org/t/355668", + source: sourceForModuleName(finalOverrideModuleName), } ); if (this.site.mobileView) { @@ -58,7 +81,6 @@ export default { // it's safe to call it original template here because the override wasn't set yet. const originalTemplate = GlimmerManager.getComponentTemplate(component); const isStrictMode = originalTemplate?.()?.parsedLayout?.isStrictMode; - const finalOverrideModuleName = moduleNames[moduleNames.length - 1]; if (isStrictMode) { const message = @@ -75,14 +97,18 @@ export default { // Special handling for these, with a different deprecation id, so the auto-feature-flag works correctly deprecated( `Overriding '${componentName}' template is deprecated. Use the value transformer 'topic-list-columns' and other new topic-list plugin APIs instead.`, - RAW_TOPIC_LIST_DEPRECATION_OPTIONS + { + ...RAW_TOPIC_LIST_DEPRECATION_OPTIONS, + source: sourceForModuleName(finalOverrideModuleName), + } ); } else { deprecated( - `[${finalOverrideModuleName}] Overriding component templates is deprecated, and will soon be disabled. Use plugin outlets, CSS, or other customization APIs instead.`, + `Overriding component templates is deprecated, and will soon be disabled. Use plugin outlets, CSS, or other customization APIs instead. [${finalOverrideModuleName}]`, { id: "discourse.component-template-overrides", - url: "https://meta.discourse.org/t/247487", + url: "https://meta.discourse.org/t/355668", + source: sourceForModuleName(finalOverrideModuleName), } ); } diff --git a/app/assets/javascripts/discourse/app/lib/deprecated.js b/app/assets/javascripts/discourse/app/lib/deprecated.js index e1c9a77bf04da..b2513c35ba6c8 100644 --- a/app/assets/javascripts/discourse/app/lib/deprecated.js +++ b/app/assets/javascripts/discourse/app/lib/deprecated.js @@ -17,7 +17,7 @@ let emberDeprecationSilencer; * @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; + const { id, since, dropFrom, url, raiseError, source } = options; if (id && disabledDeprecations.has(id)) { return; @@ -42,7 +42,8 @@ export default function deprecated(msg, options = {}) { if (require.has("discourse/lib/source-identifier")) { // This module doesn't exist in pretty-text/wizard/etc. consolePrefix = - require("discourse/lib/source-identifier").consolePrefix() || ""; + require("discourse/lib/source-identifier").consolePrefix(null, source) || + ""; } handlers.forEach((h) => h(msg, options)); diff --git a/app/assets/javascripts/discourse/app/services/deprecation-warning-handler.js b/app/assets/javascripts/discourse/app/services/deprecation-warning-handler.js index 228d9073ab427..823c9249b90a8 100644 --- a/app/assets/javascripts/discourse/app/services/deprecation-warning-handler.js +++ b/app/assets/javascripts/discourse/app/services/deprecation-warning-handler.js @@ -33,6 +33,10 @@ export const CRITICAL_DEPRECATIONS = [ "discourse.qunit.global-exists", "discourse.post-stream.trigger-new-post", "discourse.hbr-topic-list-overrides", + "discourse.mobile-templates", + "discourse.mobile-view", + "discourse.mobile-templates", + "discourse.component-template-overrides", ]; if (DEBUG) { @@ -76,7 +80,7 @@ export default class DeprecationWarningHandler extends Service { return; } - const source = identifySource(); + const source = opts.source || identifySource(); if (source?.type === "browser-extension") { return; } diff --git a/app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs b/app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs index b4c1d23689613..c37e9a57d3383 100644 --- a/app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs +++ b/app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs @@ -3,8 +3,6 @@ import { setComponentTemplate } from "@glimmer/manager"; import { render } from "@ember/test-helpers"; import { hbs } from "ember-cli-htmlbars"; import { module, test } from "qunit"; -import sinon from "sinon"; -import { overrideThrowGjsError } from "discourse/instance-initializers/component-templates"; import { withSilencedDeprecationsAsync } from "discourse/lib/deprecated"; import { forceMobile } from "discourse/lib/mobile"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; @@ -288,45 +286,4 @@ module("Integration | Initializers | plugin-component-templates", function (h) { .hasText("Resolved Theme Override", "resolved component correct"); }); }); - - module("overriding gjs component", function (hooks) { - let errorStub; - - hooks.beforeEach(() => { - registerTemporaryModule( - `discourse/components/mock-gjs-component`, - class MyComponent extends Component { - - } - ); - - registerTemporaryModule( - `discourse/plugins/my-plugin/discourse/templates/components/mock-gjs-component`, - hbs`doomed override` - ); - - errorStub = sinon - .stub(console, "error") - .withArgs(sinon.match(/mock-gjs-component was authored using gjs/)); - - overrideThrowGjsError(false); - }); - - hooks.afterEach(() => { - overrideThrowGjsError(true); - }); - - setupRenderingTest(hooks); - - test("theme overrides plugin component", async function (assert) { - await render(hbs``); - assert - .dom(".greeting") - .hasText("Hello world", "renders original implementation"); - - sinon.assert.calledOnce(errorStub); - }); - }); });