Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Deprecate `api.decoratePluginOutlet` (#24145) 

This API is not used by any known themes/plugins, and is problematic for a few reasons

- It doesn't work on modern plugin connectors which have no wrapper element

- Making modifications to Ember-rendered DOM elements can lead to catastrophic and surprising errors

- It doesn't re-run when arguments to a plugin outlet change

This commit adds the deprecation notice, and refactors the tests so that they do not rely on any real core plugin outlets
This commit is contained in:
David Taylor 2023-10-30 11:07:17 +00:00 committed by GitHub
parent 351cbab1a8
commit b7cafdc07f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/assets/javascripts/discourse/app/lib/plugin-api.js
View File

@ -1610,7 +1610,9 @@ class PluginApi {
addDocumentTitleCounter(counterFunction) {
addPluginDocumentTitleCounter(counterFunction);
}
/**
*
* Used for decorating the rendered HTML content of a plugin-outlet after it's been rendered
*
* `callback` will be called when it is time to decorate it.
@ -1628,8 +1630,14 @@ class PluginApi {
* );
* ```
*
* @deprecated because modifying an Ember-rendered DOM tree can lead to very unexpected errors. Use CSS or plugin outlet connectors instead
*
**/
decoratePluginOutlet(outletName, callback, opts) {
deprecated(
"decoratePluginOutlet is deprecated because modifying an Ember-rendered DOM tree can lead to very unexpected errors. Use CSS or plugin outlet connectors instead",
{ id: "discourse.decorate-plugin-outlet" }
);
addPluginOutletDecorator(outletName, callback, opts || {});
}

68
app/assets/javascripts/discourse/tests/acceptance/plugin-outlet-decorator-test.js
View File

@ -1,68 +0,0 @@
import { visit } from "@ember/test-helpers";
import { hbs } from "ember-cli-htmlbars";
import { test } from "qunit";
import { withPluginApi } from "discourse/lib/plugin-api";
import {
acceptance,
exists,
queryAll,
} from "discourse/tests/helpers/qunit-helpers";
import { registerTemporaryModule } from "../helpers/temporary-module-helper";
const PREFIX = "discourse/plugins/some-plugin/templates/connectors";
acceptance("Plugin Outlet - Decorator", function (needs) {
needs.user();
needs.hooks.beforeEach(() => {
registerTemporaryModule(
`${PREFIX}/discovery-list-container-top/foo`,
hbs`FOO`
);
registerTemporaryModule(
`${PREFIX}/discovery-list-container-top/bar`,
hbs`BAR`
);
withPluginApi("0.8.38", (api) => {
api.decoratePluginOutlet(
"discovery-list-container-top",
(elem, args) => {
if (elem.classList.contains("foo")) {
elem.style.backgroundColor = "yellow";
if (args.category) {
elem.classList.add("in-category");
} else {
elem.classList.remove("in-category");
}
}
},
{ id: "yellow-decorator" }
);
});
});
test("Calls the plugin callback with the rendered outlet", async function (assert) {
await visit("/");
const fooConnector = queryAll(
".discovery-list-container-top-outlet.foo "
)[0];
const barConnector = queryAll(
".discovery-list-container-top-outlet.bar "
)[0];
assert.ok(exists(fooConnector));
assert.strictEqual(fooConnector.style.backgroundColor, "yellow");
assert.strictEqual(barConnector.style.backgroundColor, "");
await visit("/c/bug");
assert.ok(fooConnector.classList.contains("in-category"));
await visit("/");
assert.notOk(fooConnector.classList.contains("in-category"));
});
});

65
app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-decorator-test.gjs Normal file
View File

@ -0,0 +1,65 @@
import { hash } from "@ember/helper";
import { render } from "@ember/test-helpers";
import { hbs } from "ember-cli-htmlbars";
import { module, test } from "qunit";
import PluginOutlet from "discourse/components/plugin-outlet";
import { withPluginApi } from "discourse/lib/plugin-api";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import { query } from "discourse/tests/helpers/qunit-helpers";
import { withSilencedDeprecations } from "discourse-common/lib/deprecated";
import { registerTemporaryModule } from "../../helpers/temporary-module-helper";
const PREFIX = "discourse/plugins/some-plugin/templates/connectors";
module("Plugin Outlet - Decorator", function (hooks) {
setupRenderingTest(hooks);
hooks.beforeEach(() => {
registerTemporaryModule(`${PREFIX}/my-outlet-name/foo`, hbs`FOO`);
registerTemporaryModule(`${PREFIX}/my-outlet-name/bar`, hbs`BAR`);
withPluginApi("0.8.38", (api) => {
withSilencedDeprecations("discourse.decorate-plugin-outlet", () => {
api.decoratePluginOutlet(
"my-outlet-name",
(elem, args) => {
if (elem.classList.contains("foo")) {
elem.style.backgroundColor = "yellow";
elem.classList.toggle("has-value", !!args.value);
}
},
{ id: "yellow-decorator" }
);
});
});
});
test("Calls the plugin callback with the rendered outlet", async function (assert) {
await render(<template>
<PluginOutlet @connectorTagName="div" @name="my-outlet-name" />
</template>);
const fooConnector = query(".my-outlet-name-outlet.foo");
const barConnector = query(".my-outlet-name-outlet.bar");
assert.dom(fooConnector).exists();
assert.strictEqual(fooConnector.style.backgroundColor, "yellow");
assert.strictEqual(barConnector.style.backgroundColor, "");
await render(<template>
<PluginOutlet
@connectorTagName="div"
@name="my-outlet-name"
@outletArgs={{hash value=true}}
/>
</template>);
assert.dom(".my-outlet-name-outlet.foo").hasClass("has-value");
await render(<template>
<PluginOutlet @connectorTagName="div" @name="my-outlet-name" />
</template>);
assert.dom(".my-outlet-name-outlet.foo").doesNotHaveClass("has-value");
});
});