From 054e9b3309dfc9710b4686418da71d25f4cd0559 Mon Sep 17 00:00:00 2001 From: takeramagan Date: Fri, 12 Jan 2024 01:31:15 -0500 Subject: [PATCH 1/3] fix createMutable getters do not work if they're on the prototype #1746 --- packages/solid/store/src/mutable.ts | 20 +++++- packages/solid/store/test/mutable.spec.ts | 81 +++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/packages/solid/store/src/mutable.ts b/packages/solid/store/src/mutable.ts index 63e92b93..0f853ae5 100644 --- a/packages/solid/store/src/mutable.ts +++ b/packages/solid/store/src/mutable.ts @@ -94,19 +94,35 @@ function wrap(value: T): T { Object.defineProperty(value, $PROXY, { value: (p = new Proxy(value, proxyTraps)) }); const keys = Object.keys(value), desc = Object.getOwnPropertyDescriptors(value); + + const proto = Object.getPrototypeOf(value); + const isClass = + value !== null && + typeof value === "object" && + !Array.isArray(value) && + proto !== Object.prototype; + if (isClass) { + const descriptors = Object.getOwnPropertyDescriptors(proto); + keys.push(...Object.keys(descriptors)); + Object.assign(desc, descriptors); + } + for (let i = 0, l = keys.length; i < l; i++) { const prop = keys[i]; + if (prop === "constructor") continue; if (desc[prop].get) { const get = desc[prop].get!.bind(p); Object.defineProperty(value, prop, { - get + get, + configurable: true }); } if (desc[prop].set) { const og = desc[prop].set!, set = (v: T[keyof T]) => batch(() => og.call(p, v)); Object.defineProperty(value, prop, { - set + set, + configurable: true }); } } diff --git a/packages/solid/store/test/mutable.spec.ts b/packages/solid/store/test/mutable.spec.ts index b8946dee..dd099606 100644 --- a/packages/solid/store/test/mutable.spec.ts +++ b/packages/solid/store/test/mutable.spec.ts @@ -316,3 +316,84 @@ describe("In Operator", () => { expect(access).toBe(0); }); }); + +describe("Class Operator test", () => { + test("read and set class", () => { + let access = 0; + class TestChildClass { + e = 10; + get d() { + return this.e * 2; + } + set d(v) { + this.e = v; + } + } + class TestClass { + a = 1; + get b() { + access++; + return access; + } + + set b(v: number) { + access = v; + } + + child = new TestChildClass(); + arr = new Array(1, 2, 3); + } + const store = createMutable<{ + a?: number; + b: number; + c?: number; + child: { e: number; d: number }; + arr: Array; + }>(new TestClass()); + + expect("a" in store).toBe(true); + expect("b" in store).toBe(true); + expect("c" in store).toBe(false); + expect(access).toBe(0); + + const [a, b, c] = createRoot(() => { + return [ + createMemo(() => "a" in store), + createMemo(() => "b" in store), + createMemo(() => "c" in store) + ]; + }); + + expect(a()).toBe(true); + expect(b()).toBe(true); + expect(c()).toBe(false); + expect(access).toBe(0); + + store.c = 3; + + expect(a()).toBe(true); + expect(b()).toBe(true); + expect(c()).toBe(true); + expect(access).toBe(0); + + delete store.a; + expect(a()).toBe(false); + expect(b()).toBe(true); + expect(c()).toBe(true); + expect(access).toBe(0); + + expect("a" in store).toBe(false); + expect("b" in store).toBe(true); + expect("c" in store).toBe(true); + expect(access).toBe(0); + + store.b = 10; + expect(access).toBe(10); + expect(store.child.d).toBe(20); + store.child.d = 20; + expect(store.child.e).toBe(20); + expect(store.child.d).toBe(40); + store.arr.push(123); + expect(store.arr).toEqual([1, 2, 3, 123]); + }); +}); From 806ae6c15bc1c146460ad687c602e07d339e2b7b Mon Sep 17 00:00:00 2001 From: takeramagan Date: Sat, 13 Jan 2024 21:20:38 -0500 Subject: [PATCH 2/3] test update for createMutable getters do not work if they're on the prototype #1746 --- packages/solid/store/src/mutable.ts | 2 +- packages/solid/store/test/mutable.spec.ts | 81 ------------------- .../store/test/mutableWithClass.spec.tsx | 55 +++++++++++++ 3 files changed, 56 insertions(+), 82 deletions(-) create mode 100644 packages/solid/store/test/mutableWithClass.spec.tsx diff --git a/packages/solid/store/src/mutable.ts b/packages/solid/store/src/mutable.ts index 0f853ae5..f88c8501 100644 --- a/packages/solid/store/src/mutable.ts +++ b/packages/solid/store/src/mutable.ts @@ -109,7 +109,7 @@ function wrap(value: T): T { for (let i = 0, l = keys.length; i < l; i++) { const prop = keys[i]; - if (prop === "constructor") continue; + if (isClass && prop === "constructor") continue; if (desc[prop].get) { const get = desc[prop].get!.bind(p); Object.defineProperty(value, prop, { diff --git a/packages/solid/store/test/mutable.spec.ts b/packages/solid/store/test/mutable.spec.ts index dd099606..b8946dee 100644 --- a/packages/solid/store/test/mutable.spec.ts +++ b/packages/solid/store/test/mutable.spec.ts @@ -316,84 +316,3 @@ describe("In Operator", () => { expect(access).toBe(0); }); }); - -describe("Class Operator test", () => { - test("read and set class", () => { - let access = 0; - class TestChildClass { - e = 10; - get d() { - return this.e * 2; - } - set d(v) { - this.e = v; - } - } - class TestClass { - a = 1; - get b() { - access++; - return access; - } - - set b(v: number) { - access = v; - } - - child = new TestChildClass(); - arr = new Array(1, 2, 3); - } - const store = createMutable<{ - a?: number; - b: number; - c?: number; - child: { e: number; d: number }; - arr: Array; - }>(new TestClass()); - - expect("a" in store).toBe(true); - expect("b" in store).toBe(true); - expect("c" in store).toBe(false); - expect(access).toBe(0); - - const [a, b, c] = createRoot(() => { - return [ - createMemo(() => "a" in store), - createMemo(() => "b" in store), - createMemo(() => "c" in store) - ]; - }); - - expect(a()).toBe(true); - expect(b()).toBe(true); - expect(c()).toBe(false); - expect(access).toBe(0); - - store.c = 3; - - expect(a()).toBe(true); - expect(b()).toBe(true); - expect(c()).toBe(true); - expect(access).toBe(0); - - delete store.a; - expect(a()).toBe(false); - expect(b()).toBe(true); - expect(c()).toBe(true); - expect(access).toBe(0); - - expect("a" in store).toBe(false); - expect("b" in store).toBe(true); - expect("c" in store).toBe(true); - expect(access).toBe(0); - - store.b = 10; - expect(access).toBe(10); - expect(store.child.d).toBe(20); - store.child.d = 20; - expect(store.child.e).toBe(20); - expect(store.child.d).toBe(40); - store.arr.push(123); - expect(store.arr).toEqual([1, 2, 3, 123]); - }); -}); diff --git a/packages/solid/store/test/mutableWithClass.spec.tsx b/packages/solid/store/test/mutableWithClass.spec.tsx new file mode 100644 index 00000000..7d3b6ad6 --- /dev/null +++ b/packages/solid/store/test/mutableWithClass.spec.tsx @@ -0,0 +1,55 @@ +/** + * @jsxImportSource solid-js + * @vitest-environment jsdom + */ +import { createMutable, unwrap, $RAW } from "../src"; +import { render, Show } from "../../web"; + +describe("Class Operator test", () => { + test("read and set class", () => { + let ref: any; + class D { + f = 1; + get e() { + return this.f * 4; + } + } + class A { + a = 1; + get b() { + return this.a * 4; + } + child = new D(); + } + let m: any; + function Test() { + m = createMutable(new A()); + m.a++; + m.child.f++; + + return ( + + ); + } + const div = document.createElement("div"); + + render(Test, div); + expect(m.b).toBe(8); + expect(m.child.e).toBe(8); + ref.$$click(); + expect(m.b).toBe(12); + expect(m.child.e).toBe(12); + ref.$$click(); + expect(m.b).toBe(16); + expect(m.child.e).toBe(16); + }); +}); From ee22b8aa2e9e2499d6808b14f1e83e1d387ae5a9 Mon Sep 17 00:00:00 2001 From: takeramagan Date: Sat, 13 Jan 2024 21:25:16 -0500 Subject: [PATCH 3/3] chore remove unused imports --- packages/solid/store/test/mutableWithClass.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/solid/store/test/mutableWithClass.spec.tsx b/packages/solid/store/test/mutableWithClass.spec.tsx index 7d3b6ad6..8f188de5 100644 --- a/packages/solid/store/test/mutableWithClass.spec.tsx +++ b/packages/solid/store/test/mutableWithClass.spec.tsx @@ -2,8 +2,8 @@ * @jsxImportSource solid-js * @vitest-environment jsdom */ -import { createMutable, unwrap, $RAW } from "../src"; -import { render, Show } from "../../web"; +import { createMutable } from "../src"; +import { render } from "../../web"; describe("Class Operator test", () => { test("read and set class", () => {