From 216845e4c73a62e3cafc6a8dfe7eb7bc4de5e55d Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 4 Nov 2024 17:38:33 +0000 Subject: [PATCH] DEV: Deprecate template overrides (#29544) Template overrides have been advised against for a long time, and are increasingly hard to maintain as Discourse's development accelerates. This commit officially deprecates this customization method, which will be removed in the not-too-distant future (likely in the first half of 2025). --- .../addon/lib/discourse-template-map.js | 26 ++++++- .../component-templates.js | 8 ++ .../integration/component-templates-test.gjs | 10 ++- .../tests/unit/ember/resolver-test.js | 77 ++++++++++--------- 4 files changed, 79 insertions(+), 42 deletions(-) diff --git a/app/assets/javascripts/discourse-common/addon/lib/discourse-template-map.js b/app/assets/javascripts/discourse-common/addon/lib/discourse-template-map.js index 2a254f54606..608a8af00a3 100644 --- a/app/assets/javascripts/discourse-common/addon/lib/discourse-template-map.js +++ b/app/assets/javascripts/discourse-common/addon/lib/discourse-template-map.js @@ -1,3 +1,5 @@ +import deprecated from "./deprecated"; + const pluginRegex = /^discourse\/plugins\/([^\/]+)\/(.*)$/; const themeRegex = /^discourse\/theme-([^\/]+)\/(.*)$/; @@ -77,12 +79,28 @@ class DiscourseTemplateMap { * theme/plugin namespaces and overrides. */ resolve(name) { - for (const cache of this.prioritizedCaches) { - const val = cache.get(name); - if (val) { - return val[val.length - 1]; + const [themeMatch, pluginMatch, coreMatch] = this.prioritizedCaches.map( + (cache) => { + const val = cache.get(name); + if (val) { + return val[val.length - 1]; + } } + ); + + if ((themeMatch || pluginMatch) && coreMatch) { + deprecated( + `[${ + themeMatch || pluginMatch + }] Overriding templates is deprecated, and will soon be disabled. Use plugin outlets, CSS, or other customization APIs instead.`, + { + id: "discourse.resolver-template-overrides", + url: "https://meta.discourse.org/t/247487", + } + ); } + + return themeMatch || pluginMatch || coreMatch; } /** 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 4bc56a1116f..28e772cdbef 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/component-templates.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/component-templates.js @@ -75,6 +75,14 @@ export default { console.error(message); } } else if (originalTemplate) { + deprecated( + `[${finalOverrideModuleName}] Overriding component templates is deprecated, and will soon be disabled. Use plugin outlets, CSS, or other customization APIs instead.`, + { + id: "discourse.component-template-overrides", + url: "https://meta.discourse.org/t/247487", + } + ); + const overrideTemplate = require(finalOverrideModuleName).default; COLOCATED_TEMPLATE_OVERRIDES.set(component, overrideTemplate); 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 5325afbb4fb..d30547db116 100644 --- a/app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs +++ b/app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs @@ -10,12 +10,16 @@ import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { withSilencedDeprecationsAsync } from "discourse-common/lib/deprecated"; import { registerTemporaryModule } from "../helpers/temporary-module-helper"; -function silenceMobileDeprecations(hooks) { +function silenceMobileAndOverrideDeprecations(hooks) { let unsilenceCallback; hooks.beforeEach(() => { const promise = new Promise((resolve) => (unsilenceCallback = resolve)); withSilencedDeprecationsAsync( - ["discourse.mobile-templates"], + [ + "discourse.mobile-templates", + "discourse.resolver-template-overrides", + "discourse.component-template-overrides", + ], () => promise ); }); @@ -112,7 +116,7 @@ function registerTemplateOnlyComponents() { } module("Integration | Initializers | plugin-component-templates", function (h) { - silenceMobileDeprecations(h); + silenceMobileAndOverrideDeprecations(h); module("template-only component definition behaviour", function (hooks) { hooks.beforeEach(() => registerTemplateOnlyComponents()); diff --git a/app/assets/javascripts/discourse/tests/unit/ember/resolver-test.js b/app/assets/javascripts/discourse/tests/unit/ember/resolver-test.js index 8a306a4f1f5..36d7b23f7cd 100644 --- a/app/assets/javascripts/discourse/tests/unit/ember/resolver-test.js +++ b/app/assets/javascripts/discourse/tests/unit/ember/resolver-test.js @@ -295,20 +295,24 @@ module("Unit | Ember | resolver", function (hooks) { ); // Defined in core and plugin - lookupTemplate( - assert, - "template:bar", - "discourse/plugins/my-plugin/discourse/templates/bar", - "prefers plugin version over core" - ); + withSilencedDeprecations("discourse.resolver-template-overrides", () => { + lookupTemplate( + assert, + "template:bar", + "discourse/plugins/my-plugin/discourse/templates/bar", + "prefers plugin version over core" + ); + }); // Defined in core and plugin and theme - lookupTemplate( - assert, - "template:baz", - "discourse/theme-12/discourse/templates/baz", - "prefers theme version over plugin and core" - ); + withSilencedDeprecations("discourse.resolver-template-overrides", () => { + lookupTemplate( + assert, + "template:baz", + "discourse/theme-12/discourse/templates/baz", + "prefers theme version over plugin and core" + ); + }); // Defined in core only lookupTemplate( @@ -330,31 +334,34 @@ module("Unit | Ember | resolver", function (hooks) { setResolverOption("mobileView", true); - withSilencedDeprecations("discourse.mobile-templates", () => { - // Default with plugin template override - lookupTemplate( - assert, - "template:foo", - "discourse/plugins/my-plugin/discourse/templates/mobile/foo", - "finding plugin version even if normal one is not present" - ); + withSilencedDeprecations( + ["discourse.mobile-templates", "discourse.resolver-template-overrides"], + () => { + // Default with plugin template override + lookupTemplate( + assert, + "template:foo", + "discourse/plugins/my-plugin/discourse/templates/mobile/foo", + "finding plugin version even if normal one is not present" + ); - // Default with plugin mobile added, takes precedence over non-mobile - lookupTemplate( - assert, - "template:bar", - "discourse/plugins/my-plugin/discourse/templates/mobile/bar", - "preferring plugin mobile version when both non-mobile plugin version is also present" - ); + // Default with plugin mobile added, takes precedence over non-mobile + lookupTemplate( + assert, + "template:bar", + "discourse/plugins/my-plugin/discourse/templates/mobile/bar", + "preferring plugin mobile version when both non-mobile plugin version is also present" + ); - // Default with when non-plugin mobile version is present - lookupTemplate( - assert, - "template:baz", - "discourse/plugins/my-plugin/discourse/templates/mobile/baz", - "preferring plugin mobile version over non-plugin mobile version" - ); - }); + // Default with when non-plugin mobile version is present + lookupTemplate( + assert, + "template:baz", + "discourse/plugins/my-plugin/discourse/templates/mobile/baz", + "preferring plugin mobile version over non-plugin mobile version" + ); + } + ); }); test("resolves templates with 'admin' prefix", function (assert) {