Skip to content

Commit 6fbedfd

Browse files
davidtaylorhqdavidtaylorhq
authored
DEV: Improve template-override deprecations (stable) (#32801)
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
1 parent 57f21d0 commit 6fbedfd

File tree

4 files changed

+38
-50
lines changed

4 files changed

+38
-50
lines changed

‎app/assets/javascripts/discourse/app/instance-initializers/component-templates.js

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import deprecated from "discourse/lib/deprecated";
44
import DiscourseTemplateMap from "discourse/lib/discourse-template-map";
55
import { isTesting } from "discourse/lib/environment";
66
import { RAW_TOPIC_LIST_DEPRECATION_OPTIONS } from "discourse/lib/plugin-api";
7+
import { getThemeInfo } from "discourse/lib/source-identifier";
78

89
let THROW_GJS_ERROR = isTesting();
910

@@ -18,6 +19,24 @@ export function overrideThrowGjsError(value) {
1819

1920
const LEGACY_TOPIC_LIST_OVERRIDES = ["topic-list", "topic-list-item"];
2021

22+
function sourceForModuleName(name) {
23+
const pluginMatch = name.match(/^discourse\/plugins\/([^\/]+)\//)?.[1];
24+
if (pluginMatch) {
25+
return {
26+
type: "plugin",
27+
name: pluginMatch,
28+
};
29+
}
30+
31+
const themeMatch = name.match(/^discourse\/theme-(\d+)\//)?.[1];
32+
if (themeMatch) {
33+
return {
34+
...getThemeInfo(parseInt(themeMatch, 10)),
35+
type: "theme",
36+
};
37+
}
38+
}
39+
2140
export default {
2241
after: ["populate-template-map", "mobile"],
2342

@@ -31,11 +50,15 @@ export default {
3150
}
3251

3352
let componentName = templateKey;
53+
const finalOverrideModuleName = moduleNames[moduleNames.length - 1];
54+
3455
if (mobile) {
3556
deprecated(
3657
`Mobile-specific hbs templates are deprecated. Use responsive CSS or {{#if this.site.mobileView}} instead. [${templateKey}]`,
3758
{
3859
id: "discourse.mobile-templates",
60+
url: "https://meta.discourse.org/t/355668",
61+
source: sourceForModuleName(finalOverrideModuleName),
3962
}
4063
);
4164
if (this.site.mobileView) {
@@ -58,7 +81,6 @@ export default {
5881
// it's safe to call it original template here because the override wasn't set yet.
5982
const originalTemplate = GlimmerManager.getComponentTemplate(component);
6083
const isStrictMode = originalTemplate?.()?.parsedLayout?.isStrictMode;
61-
const finalOverrideModuleName = moduleNames[moduleNames.length - 1];
6284

6385
if (isStrictMode) {
6486
const message =
@@ -75,14 +97,18 @@ export default {
7597
// Special handling for these, with a different deprecation id, so the auto-feature-flag works correctly
7698
deprecated(
7799
`Overriding '${componentName}' template is deprecated. Use the value transformer 'topic-list-columns' and other new topic-list plugin APIs instead.`,
78-
RAW_TOPIC_LIST_DEPRECATION_OPTIONS
100+
{
101+
...RAW_TOPIC_LIST_DEPRECATION_OPTIONS,
102+
source: sourceForModuleName(finalOverrideModuleName),
103+
}
79104
);
80105
} else {
81106
deprecated(
82-
`[${finalOverrideModuleName}] Overriding component templates is deprecated, and will soon be disabled. Use plugin outlets, CSS, or other customization APIs instead.`,
107+
`Overriding component templates is deprecated, and will soon be disabled. Use plugin outlets, CSS, or other customization APIs instead. [${finalOverrideModuleName}]`,
83108
{
84109
id: "discourse.component-template-overrides",
85-
url: "https://meta.discourse.org/t/247487",
110+
url: "https://meta.discourse.org/t/355668",
111+
source: sourceForModuleName(finalOverrideModuleName),
86112
}
87113
);
88114
}

‎app/assets/javascripts/discourse/app/lib/deprecated.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ let emberDeprecationSilencer;
1717
* @param {boolean} [options.raiseError] Raise an error when this deprecation is triggered. Defaults to `false`
1818
*/
1919
export default function deprecated(msg, options = {}) {
20-
const { id, since, dropFrom, url, raiseError } = options;
20+
const { id, since, dropFrom, url, raiseError, source } = options;
2121

2222
if (id && disabledDeprecations.has(id)) {
2323
return;
@@ -42,7 +42,8 @@ export default function deprecated(msg, options = {}) {
4242
if (require.has("discourse/lib/source-identifier")) {
4343
// This module doesn't exist in pretty-text/wizard/etc.
4444
consolePrefix =
45-
require("discourse/lib/source-identifier").consolePrefix() || "";
45+
require("discourse/lib/source-identifier").consolePrefix(null, source) ||
46+
"";
4647
}
4748

4849
handlers.forEach((h) => h(msg, options));

‎app/assets/javascripts/discourse/app/services/deprecation-warning-handler.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ export const CRITICAL_DEPRECATIONS = [
3333
"discourse.qunit.global-exists",
3434
"discourse.post-stream.trigger-new-post",
3535
"discourse.hbr-topic-list-overrides",
36+
"discourse.mobile-templates",
37+
"discourse.mobile-view",
38+
"discourse.mobile-templates",
39+
"discourse.component-template-overrides",
3640
];
3741

3842
if (DEBUG) {
@@ -76,7 +80,7 @@ export default class DeprecationWarningHandler extends Service {
7680
return;
7781
}
7882

79-
const source = identifySource();
83+
const source = opts.source || identifySource();
8084
if (source?.type === "browser-extension") {
8185
return;
8286
}

‎app/assets/javascripts/discourse/tests/integration/component-templates-test.gjs

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { setComponentTemplate } from "@glimmer/manager";
33
import { render } from "@ember/test-helpers";
44
import { hbs } from "ember-cli-htmlbars";
55
import { module, test } from "qunit";
6-
import sinon from "sinon";
7-
import { overrideThrowGjsError } from "discourse/instance-initializers/component-templates";
86
import { withSilencedDeprecationsAsync } from "discourse/lib/deprecated";
97
import { forceMobile } from "discourse/lib/mobile";
108
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
@@ -288,45 +286,4 @@ module("Integration | Initializers | plugin-component-templates", function (h) {
288286
.hasText("Resolved Theme Override", "resolved component correct");
289287
});
290288
});
291-
292-
module("overriding gjs component", function (hooks) {
293-
let errorStub;
294-
295-
hooks.beforeEach(() => {
296-
registerTemporaryModule(
297-
`discourse/components/mock-gjs-component`,
298-
class MyComponent extends Component {
299-
<template>
300-
<span class="greeting">Hello world</span>
301-
</template>
302-
}
303-
);
304-
305-
registerTemporaryModule(
306-
`discourse/plugins/my-plugin/discourse/templates/components/mock-gjs-component`,
307-
hbs`doomed override`
308-
);
309-
310-
errorStub = sinon
311-
.stub(console, "error")
312-
.withArgs(sinon.match(/mock-gjs-component was authored using gjs/));
313-
314-
overrideThrowGjsError(false);
315-
});
316-
317-
hooks.afterEach(() => {
318-
overrideThrowGjsError(true);
319-
});
320-
321-
setupRenderingTest(hooks);
322-
323-
test("theme overrides plugin component", async function (assert) {
324-
await render(hbs`<MockGjsComponent />`);
325-
assert
326-
.dom(".greeting")
327-
.hasText("Hello world", "renders original implementation");
328-
329-
sinon.assert.calledOnce(errorStub);
330-
});
331-
});
332289
});

0 commit comments

Comments
 (0)