discourse/discourse
1
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-03-09 14:34:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "DEV: Support modifyClass for native class fields on EmberObjects (#20987)" (#21080)

Browse Source 
This reverts commit a19efc4304e38a9faeef040278752a5214809d4d.

This is causing issues in production - reverting while we investigate
This commit is contained in:
David Taylor 2023-04-12 17:38:44 +01:00 committed by GitHub
parent a19efc4304
commit fa5a423681
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 19 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -227,72 +227,25 @@ class PluginApi {
* ```
**/
modifyClass(resolverName, changes, opts) {
const info = this._resolveClass(resolverName, opts);
if (!info) {
const klass = this._resolveClass(resolverName, opts);
if (!klass) {
return;
}
const klass = info.class;
if (canModify(info, "member", resolverName, changes)) {
if (canModify(klass, "member", resolverName, changes)) {
delete changes.pluginId;
const changeDescriptors = Object.getOwnPropertyDescriptors(changes);
const fieldDescriptorEntries = Object.entries(changeDescriptors).filter(
([key, desc]) => {
if (typeof desc.value === "function" || desc.get || desc.set) {
return false;
}
if (
klass.prototype.mergedProperties?.includes(key) ||
klass.prototype.concatenatedProperties?.includes(key)
) {
return false;
}
return true;
}
);
if (klass.reopen) {
// klass is an EmberObject - use builtin ember 'reopen' feature
klass.reopen(changes);
if (fieldDescriptorEntries.length) {
// reopen sets these values on the prototype, but they might be overrides for
// native class fields. Those need to be overridden after the original values
// have been set on the instance by the constructor.
klass.reopen({
init() {
Object.defineProperties(
this,
Object.fromEntries(fieldDescriptorEntries)
);
this._super();
},
});
}
if (klass.class.reopen) {
klass.class.reopen(changes);
} else {
// Not an EmberObject. We can override functions/getters/setters on the prototype
// but we don't currently have a way to override fields. Log a warning to the console
// if there are any overrides that look like fields.
Object.defineProperties(klass.prototype, changeDescriptors);
if (fieldDescriptorEntries.length) {
// eslint-disable-next-line no-console
console.warn(
consolePrefix(),
`Attempted to modify fields in ${resolverName}, which is not an EmberObject. ` +
"These changes will not take effect. " +
`Fields: ${JSON.stringify(
fieldDescriptorEntries.map(([key]) => key)
)}`
);
}
Object.defineProperties(
klass.class.prototype || klass.class,
Object.getOwnPropertyDescriptors(changes)
);
}
}
return info;
return klass;
}
/**

46
app/assets/javascripts/discourse/tests/unit/lib/plugin-api-test.js
View File

@ -4,7 +4,6 @@ import discourseComputed from "discourse-common/utils/decorators";
import { withPluginApi } from "discourse/lib/plugin-api";
import { setupTest } from "ember-qunit";
import { getOwner } from "discourse-common/lib/get-owner";
import Sinon from "sinon";
module("Unit | Utility | plugin-api", function (hooks) {
setupTest(hooks);
@ -36,9 +35,6 @@ module("Unit | Utility | plugin-api", function (hooks) {
test("modifyClass works with native class Ember objects", function (assert) {
class NativeTestThingy extends EmberObject {
firstField = "firstFieldValue";
otherField = "otherFieldValue";
@discourseComputed
prop() {
return "howdy";
@ -51,8 +47,6 @@ module("Unit | Utility | plugin-api", function (hooks) {
api.modifyClass("native-test-thingy:main", {
pluginId: "plugin-api-test",
otherField: "new otherFieldValue",
@discourseComputed
prop() {
return `${this._super(...arguments)} partner`;
@ -62,15 +56,10 @@ module("Unit | Utility | plugin-api", function (hooks) {
const thingy = getOwner(this).lookup("native-test-thingy:main");
assert.strictEqual(thingy.prop, "howdy partner");
assert.strictEqual(thingy.firstField, "firstFieldValue");
assert.strictEqual(thingy.otherField, "new otherFieldValue");
});
test("modifyClass works with native classes", function (assert) {
class ClassTestThingy {
firstField = "firstFieldValue";
otherField = "otherFieldValue";
get keep() {
return "hey!";
}
@ -80,42 +69,23 @@ module("Unit | Utility | plugin-api", function (hooks) {
}
}
getOwner(this).register("class-test-thingy:main", ClassTestThingy);
const warnStub = Sinon.stub(console, "warn");
getOwner(this).register("class-test-thingy:main", new ClassTestThingy(), {
instantiate: false,
});
withPluginApi("1.1.0", (api) => {
api.modifyClass("class-test-thingy:main", {
pluginId: "plugin-api-test",
otherField: "new otherFieldValue",
get prop() {
return "g'day";
},
});
});
assert.strictEqual(
warnStub.callCount,
1,
"fields warning was printed to console"
);
assert.true(warnStub.args[0][1].startsWith("Attempted to modify fields"));
const thingy = new ClassTestThingy();
assert.strictEqual(thingy.keep, "hey!", "maintains unchanged base getter");
assert.strictEqual(thingy.prop, "g'day", "can override getter");
assert.strictEqual(
thingy.firstField,
"firstFieldValue",
"maintains unchanged base field"
);
assert.strictEqual(
thingy.otherField,
"otherFieldValue",
"cannot override field"
);
const thingy = getOwner(this).lookup("class-test-thingy:main");
assert.strictEqual(thingy.keep, "hey!");
assert.strictEqual(thingy.prop, "g'day");
});
skip("modifyClass works with getters", function (assert) {
@ -125,7 +95,9 @@ module("Unit | Utility | plugin-api", function (hooks) {
},
});
getOwner(this).register("test-class:main", Base);
getOwner(this).register("test-class:main", Base, {
instantiate: false,
});
// Performing this lookup triggers `factory._onLookup`. In DEBUG builds, that invokes injectedPropertyAssertion()
// https://github.com/emberjs/ember.js/blob/36505f1b42/packages/%40ember/-internals/runtime/lib/system/core_object.js#L1144-L1163