From 697a912d159e78f6e27c54894ce7bbeee06b0b8c Mon Sep 17 00:00:00 2001 From: Cold Fry Date: Thu, 26 Mar 2026 14:29:30 +0000 Subject: [PATCH 1/2] fix Object.defineProperty sharing values across instances When Object.defineProperty is called on an instance (not a prototype), create a per-instance metatable if the shared metatable already has descriptors, so that each instance gets its own descriptor storage. Fixes #1625 --- src/lualib/SetDescriptor.ts | 13 +++++++++++++ test/unit/builtins/object.spec.ts | 15 +++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/lualib/SetDescriptor.ts b/src/lualib/SetDescriptor.ts index 4339c8cc1..a8889bfb2 100644 --- a/src/lualib/SetDescriptor.ts +++ b/src/lualib/SetDescriptor.ts @@ -26,6 +26,19 @@ export function __TS__SetDescriptor( setmetatable(target, metatable); } + // When setting a descriptor on an instance (not a prototype), ensure it has + // its own metatable so descriptors are not shared across instances. + // See: https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1625 + if (!isPrototype && rawget(metatable, "_descriptors")) { + // The metatable already has descriptors from a previous defineProperty + // call (likely on a different instance sharing the same class metatable). + // Create a per-instance metatable that chains to the original. + const instanceMetatable: any = {}; + setmetatable(instanceMetatable, metatable); + setmetatable(target, instanceMetatable); + metatable = instanceMetatable; + } + const value = rawget(target, key); if (value !== undefined) rawset(target, key, undefined); diff --git a/test/unit/builtins/object.spec.ts b/test/unit/builtins/object.spec.ts index 1edc8a9d6..13f6967e7 100644 --- a/test/unit/builtins/object.spec.ts +++ b/test/unit/builtins/object.spec.ts @@ -196,6 +196,21 @@ describe("Object.defineProperty", () => { return { prop: foo.bar, err }; `.expectToMatchJsResult(); }); + + // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1625 + test("instance isolation", () => { + util.testFunction` + class Test { + declare obj: object; + constructor() { + Object.defineProperty(this, "obj", { value: {}, writable: true, configurable: true }); + } + } + const t1 = new Test(); + const t2 = new Test(); + return t1.obj === t2.obj; + `.expectToMatchJsResult(); + }); }); describe("Object.getOwnPropertyDescriptor", () => { From da8f1351f569a71fd5faf540f88de28927a7928b Mon Sep 17 00:00:00 2001 From: Cold Fry Date: Fri, 27 Mar 2026 17:26:56 +0000 Subject: [PATCH 2/2] fix Object.defineProperty instance isolation for first instance Use a marker to always create per-instance metatables, not just when _descriptors already exists on the shared metatable. Add more thorough instance isolation tests. --- src/lualib/SetDescriptor.ts | 6 ++-- test/unit/builtins/object.spec.ts | 60 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/lualib/SetDescriptor.ts b/src/lualib/SetDescriptor.ts index a8889bfb2..ccb4bf3f0 100644 --- a/src/lualib/SetDescriptor.ts +++ b/src/lualib/SetDescriptor.ts @@ -29,11 +29,9 @@ export function __TS__SetDescriptor( // When setting a descriptor on an instance (not a prototype), ensure it has // its own metatable so descriptors are not shared across instances. // See: https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1625 - if (!isPrototype && rawget(metatable, "_descriptors")) { - // The metatable already has descriptors from a previous defineProperty - // call (likely on a different instance sharing the same class metatable). - // Create a per-instance metatable that chains to the original. + if (!isPrototype && !rawget(metatable, "_isOwnDescriptorMetatable")) { const instanceMetatable: any = {}; + instanceMetatable._isOwnDescriptorMetatable = true; setmetatable(instanceMetatable, metatable); setmetatable(target, instanceMetatable); metatable = instanceMetatable; diff --git a/test/unit/builtins/object.spec.ts b/test/unit/builtins/object.spec.ts index 13f6967e7..dfed73502 100644 --- a/test/unit/builtins/object.spec.ts +++ b/test/unit/builtins/object.spec.ts @@ -211,6 +211,66 @@ describe("Object.defineProperty", () => { return t1.obj === t2.obj; `.expectToMatchJsResult(); }); + + test("instance isolation with three instances", () => { + util.testFunction` + class Test { + declare obj: object; + constructor() { + Object.defineProperty(this, "obj", { value: {}, writable: true, configurable: true }); + } + } + const t1 = new Test(); + const t2 = new Test(); + const t3 = new Test(); + return [t1.obj === t2.obj, t1.obj === t3.obj, t2.obj === t3.obj]; + `.expectToMatchJsResult(); + }); + + test("instance isolation with mutation", () => { + util.testFunction` + class Test { + declare value: number; + constructor(v: number) { + Object.defineProperty(this, "value", { value: v, writable: true, configurable: true }); + } + } + const t1 = new Test(1); + const t2 = new Test(2); + return [t1.value, t2.value]; + `.expectToMatchJsResult(); + }); + + test("instance isolation with multiple properties", () => { + util.testFunction` + class Test { + declare a: string; + declare b: string; + constructor(a: string, b: string) { + Object.defineProperty(this, "a", { value: a, writable: true, configurable: true }); + Object.defineProperty(this, "b", { value: b, writable: true, configurable: true }); + } + } + const t1 = new Test("x", "y"); + const t2 = new Test("p", "q"); + return [t1.a, t1.b, t2.a, t2.b]; + `.expectToMatchJsResult(); + }); + + test("instance isolation preserves prototype methods", () => { + util.testFunction` + class Test { + declare val: number; + constructor(v: number) { + Object.defineProperty(this, "val", { value: v, writable: true, configurable: true }); + } + getVal() { return this.val; } + } + const t1 = new Test(10); + const t2 = new Test(20); + return [t1.getVal(), t2.getVal()]; + `.expectToMatchJsResult(); + }); }); describe("Object.getOwnPropertyDescriptor", () => {