From 613dea61a2b5d437a4c9f08b0f151effb38ae284 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Mon, 25 Nov 2024 22:48:00 +0100 Subject: [PATCH] DEV: Allow ember hash as a context in applyValueTransformer (#29922) This unlocks the ability to use that function directly in templates: ```hbs {{applyValueTransformer "foo-bar" @defaultValue (hash arg1=@arg1 arg2=@arg2) }} ``` --- .../discourse/app/lib/transformer.js | 7 +++- .../tests/unit/lib/transformer-test.js | 41 +++++++++++-------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/transformer.js b/app/assets/javascripts/discourse/app/lib/transformer.js index 477879eeff9..c04790b8e9b 100644 --- a/app/assets/javascripts/discourse/app/lib/transformer.js +++ b/app/assets/javascripts/discourse/app/lib/transformer.js @@ -317,10 +317,13 @@ export function applyValueTransformer( if ( typeof (context ?? undefined) !== "undefined" && - !(typeof context === "object" && context.constructor === Object) + !( + typeof context === "object" && + (context.constructor === Object || context.constructor === undefined) + ) ) { throw ( - `${prefix}("${transformerName}", ...): context must be a simple JS object or nullish.\n` + + `${prefix}("${transformerName}", ...): context must be a simple JS object/an Ember hash or nullish.\n` + "Avoid passing complex objects in the context, like for example, component instances or objects that carry " + "mutable state directly. This can induce users to registry transformers with callbacks causing side effects " + "and mutating the context directly. Inevitably, this leads to fragile integrations." diff --git a/app/assets/javascripts/discourse/tests/unit/lib/transformer-test.js b/app/assets/javascripts/discourse/tests/unit/lib/transformer-test.js index e946d583018..341e7d215b6 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/transformer-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/transformer-test.js @@ -14,6 +14,15 @@ import { transformerWasAdded, } from "discourse/lib/transformer"; +function notThrows(testCallback) { + try { + testCallback(); + return true; + } catch { + return false; + } +} + module("Unit | Utility | transformers", function (hooks) { setupTest(hooks); @@ -208,15 +217,6 @@ module("Unit | Utility | transformers", function (hooks) { }); test("accepts only simple objects as context", function (assert) { - const notThrows = (testCallback) => { - try { - testCallback(); - return true; - } catch { - return false; - } - }; - assert.ok( notThrows(() => applyValueTransformer("test-value1-transformer", "foo") @@ -317,6 +317,20 @@ module("Unit | Utility | transformers", function (hooks) { ); }); + test("accepts an ember hash proxy as context", function (assert) { + // A hash-like object + const hash = { a: 1 }; + Object.setPrototypeOf(hash, null); + const proxy = new Proxy(hash, {}); + + assert.true( + notThrows(() => + applyValueTransformer("test-value1-transformer", "foo", proxy) + ), + "doesn't throw an error if context is a proxy to an ember hash object" + ); + }); + test("applying the transformer works", function (assert) { class Testable { #value; @@ -777,15 +791,6 @@ module("Unit | Utility | transformers", function (hooks) { }); test("accepts only simple objects as context", function (assert) { - const notThrows = (testCallback) => { - try { - testCallback(); - return true; - } catch { - return false; - } - }; - assert.ok( notThrows(() => applyBehaviorTransformer("test-behavior1-transformer", () => true)