DEV: Log error to console when attempting to override gjs template (#23513)

When using ember-template-tag (.gjs), templates are much more interlinked with the JS file they're defined in. Therefore, attempting to override their template with a 'non-strict-mode' template doesn't make sense, and will likely lead to problems.

This commit skips any such overrides, and introduces a clear console warning. In theme/plugin tests, an error will be thrown during app boot.

Going forward, we aim to provide alternative APIs to achieve the customizations people currently implement with template overrides. (e.g. https://github.com/discourse/discourse/pull/23110)
This commit is contained in:
David Taylor 2023-09-11 19:21:44 +01:00 committed by GitHub
parent 70d082584c
commit c2cad3f269
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 71 additions and 6 deletions

View File

@ -1,9 +1,17 @@
import DiscourseTemplateMap from "discourse-common/lib/discourse-template-map";
import * as GlimmerManager from "@glimmer/manager";
import ClassicComponent from "@ember/component";
import { isTesting } from "discourse-common/config/environment";
const COLOCATED_TEMPLATE_OVERRIDES = new Map();
let THROW_GJS_ERROR = isTesting();
/** For use in tests/integration/component-templates-test only */
export function overrideThrowGjsError(value) {
THROW_GJS_ERROR = value;
}
// This patch is not ideal, but Ember does not allow us to change a component template after initial association
// https://github.com/glimmerjs/glimmer-vm/blob/03a4b55c03/packages/%40glimmer/manager/lib/public/template.ts#L14-L20
const originalGetTemplate = GlimmerManager.getComponentTemplate;
@ -34,15 +42,29 @@ export default {
const component = owner.resolveRegistration(`component:${componentName}`);
if (component && originalGetTemplate(component)) {
const finalOverrideModuleName = moduleNames[moduleNames.length - 1];
const overrideTemplate = require(finalOverrideModuleName).default;
COLOCATED_TEMPLATE_OVERRIDES.set(component, overrideTemplate);
} else if (!component) {
if (!component) {
// Plugin/theme component template with no backing class.
// Treat as classic component to emulate pre-template-only-glimmer-component behaviour.
owner.register(`component:${componentName}`, ClassicComponent);
return;
}
const originalTemplate = originalGetTemplate(component);
const isStrictMode = originalTemplate?.()?.parsedLayout?.isStrictMode;
const finalOverrideModuleName = moduleNames[moduleNames.length - 1];
if (isStrictMode) {
const message = `[${finalOverrideModuleName}] ${componentName} was authored using gjs and its template cannot be overridden. Ignoring override.`;
if (THROW_GJS_ERROR) {
throw new Error(message);
} else {
// eslint-disable-next-line no-console
console.error(message);
}
} else if (originalTemplate) {
const overrideTemplate = require(finalOverrideModuleName).default;
COLOCATED_TEMPLATE_OVERRIDES.set(component, overrideTemplate);
}
});
},

View File

@ -6,6 +6,8 @@ import { registerTemporaryModule } from "../helpers/temporary-module-helper";
import { setComponentTemplate } from "@glimmer/manager";
import Component from "@glimmer/component";
import { forceMobile, resetMobile } from "discourse/lib/mobile";
import sinon from "sinon";
import { overrideThrowGjsError } from "discourse/instance-initializers/component-templates";
class MockColocatedComponent extends Component {}
setComponentTemplate(hbs`Colocated Original`, MockColocatedComponent);
@ -266,4 +268,45 @@ module("Integration | Initializers | plugin-component-templates", function () {
.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 {
<template>
<span class="greeting">Hello world</span>
</template>
}
);
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 () {
await render(hbs`<MockGjsComponent />`);
assert
.dom(".greeting")
.hasText("Hello world", "renders original implementation");
sinon.assert.calledOnce(errorStub);
});
});
});