From 88412a2d2bd09eb52f6ffb735c2e06e223e814e6 Mon Sep 17 00:00:00 2001 From: "Michael (mcm)" Date: Fri, 8 Jul 2022 07:14:01 +0000 Subject: [PATCH] [FIX] web: fix patch descriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, patching a getter of a class made it enumerable (`Object.keys({ a: 1 })` -> `["a"]`). In javascript, getters of a class are not enumerable but getters of an object are enumerable. When patching, the system replaced the descriptor of the old class member by the member of the object. Now, it replaces some info of the old descriptor by the new descriptor but not the entire descriptor. closes odoo/odoo#95921 X-original-commit: fd7c2c0c14bfaac228abacb265278157eec4aad0 Signed-off-by: Géry Debongnie Signed-off-by: Michaël Mattiello --- addons/web/static/src/core/utils/patch.js | 10 +++++--- .../static/tests/core/utils/patch_tests.js | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/addons/web/static/src/core/utils/patch.js b/addons/web/static/src/core/utils/patch.js index 8d27121625a..e23941e6310 100644 --- a/addons/web/static/src/core/utils/patch.js +++ b/addons/web/static/src/core/utils/patch.js @@ -39,7 +39,7 @@ export function patch(obj, patchName, patchValue, options = {}) { proto = Object.getPrototypeOf(proto); } while (!prevDesc && proto); - const newDesc = Object.getOwnPropertyDescriptor(patchValue, k); + let newDesc = Object.getOwnPropertyDescriptor(patchValue, k); if (!objDesc.original.hasOwnProperty(k)) { objDesc.original[k] = Object.getOwnPropertyDescriptor(obj, k); } @@ -48,6 +48,7 @@ export function patch(obj, patchName, patchValue, options = {}) { const patchedFnName = `${k} (patch ${patchName})`; if (prevDesc.value && typeof newDesc.value === "function") { + newDesc = { ...prevDesc, value: newDesc.value }; makeIntermediateFunction("value", prevDesc, newDesc, patchedFnName); } if ((newDesc.get || newDesc.set) && (prevDesc.get || prevDesc.set)) { @@ -55,8 +56,11 @@ export function patch(obj, patchName, patchValue, options = {}) { // in the previous descriptor but only one in the new descriptor // then the other will be undefined so we need to apply the // previous descriptor in the new one. - newDesc.get = newDesc.get || prevDesc.get; - newDesc.set = newDesc.set || prevDesc.set; + newDesc = { + ...prevDesc, + get: newDesc.get || prevDesc.get, + set: newDesc.set || prevDesc.set, + }; if (prevDesc.get && typeof newDesc.get === "function") { makeIntermediateFunction("get", prevDesc, newDesc, patchedFnName); } diff --git a/addons/web/static/tests/core/utils/patch_tests.js b/addons/web/static/tests/core/utils/patch_tests.js index 20e8213af42..f8d460d6813 100644 --- a/addons/web/static/tests/core/utils/patch_tests.js +++ b/addons/web/static/tests/core/utils/patch_tests.js @@ -1085,6 +1085,30 @@ QUnit.module("utils", () => { assert.verifySteps(["base.fn", "extension.fn"]); }); + QUnit.test("keep original descriptor details", async function (assert) { + class BaseClass { + // getter declared in classes are not enumerable + get getter() { + return false; + } + } + let descriptor = Object.getOwnPropertyDescriptor(BaseClass.prototype, "getter"); + let getterFn = descriptor.get; + assert.strictEqual(descriptor.configurable, true); + assert.strictEqual(descriptor.enumerable, false); + + patch(BaseClass.prototype, "patch", { + // getter declared in object are enumerable + get getter() { + return true; + }, + }); + descriptor = Object.getOwnPropertyDescriptor(BaseClass.prototype, "getter"); + assert.strictEqual(descriptor.configurable, true); + assert.strictEqual(descriptor.enumerable, false); + assert.notStrictEqual(getterFn, descriptor.get); + }); + QUnit.module("other"); QUnit.test("patch an object", async function (assert) {