perf(dsl): skip placeholder pattern in materializeClassInstance

examples/js_dsl/mod.test.ts

@@ -391,7 +391,7 @@ describe("optional parameters", () => {
 });
 
 describe("class materialization", () => {
-	it("static class return avoids constructor placeholder allocation", () => {
+	it("static class return avoids constructor init allocation", () => {
 		const initBefore = mod.getFactoryResourceInitCount();
 		const deinitBefore = mod.getFactoryResourceDeinitCount();
 
@@ -402,7 +402,7 @@ describe("class materialization", () => {
 		expect(mod.getFactoryResourceDeinitCount()).toEqual(deinitBefore);
 	});
 
-	it("instance class return avoids constructor placeholder allocation", () => {
+	it("instance class return avoids constructor init allocation", () => {
 		const base = mod.FactoryResource.withByte(1);
 		const initBefore = mod.getFactoryResourceInitCount();
 		const deinitBefore = mod.getFactoryResourceDeinitCount();
@@ -425,6 +425,62 @@ describe("class materialization", () => {
 	it("rejects cross-class static factory binding during materialization", () => {
 		expect(() => mod.Point.create.call(mod.Buffer, 1, 2)).toThrow();
 	});
+
+	it("preserves normal nested construction inside subclass constructors", () => {
+		// Same-class materialization may call a JS subclass constructor via the preferred
+		// receiver constructor. A nested normal `new` inside that constructor must not
+		// inherit the internal materialization marker, otherwise it skips native wrapping.
+		let nested: InstanceType<typeof mod.FactoryResource> | undefined;
+		class DerivedFactoryResource extends mod.FactoryResource {
+			constructor() {
+				super();
+				nested = new mod.FactoryResource();
+			}
+		}
+
+		const resource = DerivedFactoryResource.withByte(5);
+
+		expect(resource).toBeInstanceOf(DerivedFactoryResource);
+		expect(resource.getByte()).toEqual(5);
+		expect(nested?.getByte()).toEqual(0);
+	});
+
+	it("rejects subclass constructors that return a different object", () => {
+		class ReplacingPoint extends mod.Point {
+			constructor() {
+				super();
+				return {};
+			}
+		}
+
+		expect(() => ReplacingPoint.create(3, 4)).toThrow();
+	});
+
+	it("does not run cross-class constructors during failed materialization", () => {
+		const initBefore = mod.getFactoryResourceInitCount();
+		const deinitBefore = mod.getFactoryResourceDeinitCount();
+
+		expect(() => mod.Point.create.call(mod.FactoryResource, 1, 2)).toThrow();
+
+		expect(mod.getFactoryResourceInitCount()).toEqual(initBefore);
+		expect(mod.getFactoryResourceDeinitCount()).toEqual(deinitBefore);
+	});
+
+	it("rejects non-zapi constructors during materialization", () => {
+		function FakePoint() {}
+
+		expect(() => mod.Point.create.call(FakePoint, 1, 2)).toThrow();
+	});
+
+	it("deinitializes returned native resources when materialization fails", () => {
+		const initBefore = mod.getFactoryResourceInitCount();
+		const deinitBefore = mod.getFactoryResourceDeinitCount();
+
+		expect(() => mod.FactoryResource.withByte.call(mod.Point, 7)).toThrow();
+
+		expect(mod.getFactoryResourceInitCount()).toEqual(initBefore + 1);
+		expect(mod.getFactoryResourceDeinitCount()).toEqual(deinitBefore + 1);
+	});
 });
 
 // Section 15: Getters and Setters

examples/js_dsl/mod.zig

@@ -426,7 +426,7 @@ pub const Point = struct {
     }
 };
 
-/// A resource-owning class used to verify placeholder cleanup in factory paths.
+/// A resource-owning class used to verify cleanup in class materialization paths.
 pub const FactoryResource = struct {
     pub const js_meta = js.class(.{});
     data: []u8,

src/js/class_runtime.zig

@@ -8,15 +8,11 @@ pub fn typeTag(comptime T: type) napi.c.napi_type_tag {
     };
 }
 
-pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T, finalize_hint: ?*anyopaque) !void {
+pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T) !void {
     const tag = typeTag(T);
-    try env.wrap(object, T, native_object, defaultFinalize(T), finalize_hint, null);
+    try env.wrap(object, T, native_object, defaultFinalize(T), null, null);
     errdefer if (env.removeWrap(T, object)) |removed| {
-        if (isInternalPlaceholderHint(T, finalize_hint)) {
-            destroyInternalPlaceholder(T, removed);
-        } else {
-            destroyNativeObject(T, removed);
-        }
+        destroyNativeObject(T, removed);
     } else |_| {};
     if (!(try env.checkObjectTypeTag(object, tag))) {
         try env.typeTagObject(object, tag);
@@ -48,17 +44,9 @@ pub fn destroyNativeObject(comptime T: type, obj: *T) void {
     std.heap.c_allocator.destroy(obj);
 }
 
-pub fn destroyInternalPlaceholder(comptime T: type, obj: *T) void {
-    std.heap.c_allocator.destroy(obj);
-}
-
 pub fn defaultFinalize(comptime T: type) napi.FinalizeCallback(T) {
     return struct {
-        fn f(_: napi.Env, obj: *T, hint: ?*anyopaque) void {
-            if (isInternalPlaceholderHint(T, hint)) {
-                destroyInternalPlaceholder(T, obj);
-                return;
-            }
+        fn f(_: napi.Env, obj: *T, _: ?*anyopaque) void {
             destroyNativeObject(T, obj);
         }
     }.f;
@@ -85,28 +73,75 @@ pub fn registerClass(comptime T: type, env: napi.Env, ctor: napi.Value) !void {
     try env.addEnvCleanupHook(State.Entry, entry, State.cleanupHook);
 }
 
+/// Per-thread marker set by `materializeClassInstance` to tell the generated
+/// constructor "this `new` call comes from the DSL; return the JS instance
+/// without running `init`, and materialization will wrap the native object."
+/// Compared by identity against `internalCtorMarkerPtr(T)`.
+threadlocal var materialize_target: ?*const anyopaque = null;
+
+/// Captures the exact `this` object whose generated base constructor consumed
+/// `materialize_target`. JS derived constructors are allowed to `return {}`
+/// after `super()`, causing `napi_new_instance` to return that replacement
+/// object. Materialization must reject that case instead of wrapping native
+/// state onto an unrelated object with the wrong prototype.
+///
+/// Stored as a temporary N-API reference because nested JS construction can run
+/// before `napi_new_instance` returns; keeping only the raw constructor callback
+/// handle is not stable enough across that nested call stack.
+threadlocal var materialized_instance: ?napi.Ref = null;
+
+pub fn isMaterializing(comptime T: type) bool {
+    return materialize_target == @as(?*const anyopaque, @ptrCast(internalCtorMarkerPtr(T)));
+}
+
+pub fn hasPendingMaterialization() bool {
+    return materialize_target != null;
+}
+
+pub fn consumeMaterialization(comptime T: type, env: napi.Env, this_arg: napi.c.napi_value) !bool {
+    if (!isMaterializing(T)) return false;
+    const this_val = napi.Value{ .env = env.env, .value = this_arg };
+    const this_ref = try env.createReference(this_val, 1);
+    materialize_target = null;
+    materialized_instance = this_ref;
+    return true;
+}
+
 pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, preferred_ctor: ?napi.Value) !napi.Value {
     const ctor = preferred_ctor orelse try getConstructor(T, env);
-    const internal_arg = try env.createExternal(@ptrCast(internalCtorMarkerPtr(T)), null, null);
-    var raw_args = [_]napi.c.napi_value{internal_arg.value};
+
+    const obj_ptr = try std.heap.c_allocator.create(T);
+    errdefer destroyNativeObject(T, obj_ptr);
+    obj_ptr.* = instance;
+
+    const prev = materialize_target;
+    const prev_instance = materialized_instance;
+    materialize_target = @ptrCast(internalCtorMarkerPtr(T));
+    materialized_instance = null;
+    defer materialize_target = prev;
+    defer {
+        if (materialized_instance) |ref| ref.delete() catch {};
+        materialized_instance = prev_instance;
+    }
 
     var js_instance_raw: napi.c.napi_value = null;
     try napi.status.check(napi.c.napi_new_instance(
         env.env,
         ctor.value,
-        1,
-        &raw_args,
+        0,
+        null,
         &js_instance_raw,
     ));
 
     const js_instance = napi.Value{ .env = env.env, .value = js_instance_raw };
-    const placeholder = try env.removeWrapChecked(T, js_instance, typeTag(T));
-    destroyInternalPlaceholder(T, placeholder);
-
-    const obj_ptr = try std.heap.c_allocator.create(T);
-    obj_ptr.* = instance;
-
-    try wrapTaggedObject(T, env, js_instance, obj_ptr, null);
+    if (materialize_target != null) return error.InvalidMaterializationConstructor;
+    const expected_instance_ref = materialized_instance orelse return error.InvalidMaterializationConstructor;
+    const expected_instance = try expected_instance_ref.getValue();
+    // The generated constructor must be the object that comes back from
+    // `napi_new_instance`; otherwise a subclass returned a replacement object.
+    if (!(try expected_instance.strictEquals(js_instance))) return error.InvalidMaterializationConstructor;
+
+    try wrapTaggedObject(T, env, js_instance, obj_ptr);
     return js_instance;
 }
 
@@ -120,19 +155,6 @@ fn getConstructor(comptime T: type, env: napi.Env) !napi.Value {
     return try entry.ctor_ref.getValue();
 }
 
-pub fn isInternalCtorArg(comptime T: type, value: napi.Value) bool {
-    const raw = value.getValueExternal() catch return false;
-    return raw == @as(*anyopaque, @ptrCast(internalCtorMarkerPtr(T)));
-}
-
-pub fn internalPlaceholderHint(comptime T: type) ?*anyopaque {
-    return @ptrCast(&markers(T).placeholder_hint);
-}
-
-pub fn isInternalPlaceholderHint(comptime T: type, hint: ?*anyopaque) bool {
-    return hint == internalPlaceholderHint(T);
-}
-
 fn state(comptime T: type) type {
     return struct {
         const Class = T;
@@ -193,14 +215,9 @@ fn markers(comptime T: type) type {
         }
 
         var ctor_marker: u8 = 0;
-        var placeholder_hint: u8 = 0;
     };
 }
 
-fn internalCtorMarker(comptime T: type) [*]const u8 {
-    return internalCtorMarkerPtr(T);
-}
-
 fn internalCtorMarkerPtr(comptime T: type) *u8 {
     return &markers(T).ctor_marker;
 }

src/js/wrap_class.zig

@@ -68,21 +68,15 @@ pub fn wrapClass(comptime T: type) type {
         /// This callback is invoked when `new Class(...)` is called in JavaScript.
         /// It handles argument conversion, calls the `pub fn init(...)` method of
         /// the Zig struct `T`, and wraps the resulting native object in the JS instance.
-        /// It also handles internal placeholder creation for factory methods.
+        /// It also supports direct wrapping for DSL-returned class instances.
         pub const constructor: napi.c.napi_callback = genConstructor();
 
         /// The default N-API finalizer callback for native objects wrapped by instances of `T`.
         ///
         /// This function is registered with `napi.Env.wrap()` and is called by the
         /// JavaScript garbage collector when the wrapped JS object is finalized.
-        /// It handles cleanup for internal placeholder objects during class
-        /// materialization and calls the `deinit()` method (if present) and frees
-        /// the native memory for regular class instances.
-        pub fn defaultFinalize(_: napi.Env, obj: *T, hint: ?*anyopaque) void {
-            if (class_runtime.isInternalPlaceholderHint(T, hint)) {
-                class_runtime.destroyInternalPlaceholder(T, obj);
-                return;
-            }
+        /// It calls the `deinit()` method (if present) and frees the native memory.
+        pub fn defaultFinalize(_: napi.Env, obj: *T, _: ?*anyopaque) void {
             class_runtime.destroyNativeObject(T, obj);
         }
 
@@ -366,22 +360,19 @@ pub fn wrapClass(comptime T: type) type {
                         return null;
                     };
 
-                    if (actual_argc == 1) {
-                        const internal_arg = napi.Value{ .env = raw_env, .value = raw_args[0] };
-                        if ((internal_arg.typeof() catch null) == .external) {
-                            const obj_ptr = std.heap.c_allocator.create(T) catch {
-                                e.throwError("", "Out of memory allocating internal placeholder") catch {};
-                                return null;
-                            };
-                            obj_ptr.* = std.mem.zeroes(T);
-
-                            const this_val = napi.Value{ .env = raw_env, .value = this_arg };
-                            class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr, class_runtime.internalPlaceholderHint(T)) catch {
-                                e.throwError("", "Failed to wrap internal placeholder") catch {};
-                                return null;
-                            };
-                            return this_arg;
-                        }
+                    // Fast path: materializeClassInstance is creating this instance.
+                    // Skip normal init; materialize will wrap the returned JS instance
+                    // with the real native pointer after napi_new_instance returns.
+                    const did_consume_materialization = class_runtime.consumeMaterialization(T, e, this_arg) catch {
+                        e.throwError("", "Failed to record materialized instance") catch {};
+                        return null;
+                    };
+                    if (did_consume_materialization) {
+                        return this_arg;
+                    }
+                    if (class_runtime.hasPendingMaterialization()) {
+                        e.throwTypeError("", "Invalid materialization constructor") catch {};
+                        return null;
                     }
 
                     const required_init_argc = comptime wrap_function.requiredArgCount(init_params);
@@ -408,7 +399,7 @@ pub fn wrapClass(comptime T: type) type {
                     obj_ptr.* = init_result;
 
                     const this_val = napi.Value{ .env = raw_env, .value = this_arg };
-                    class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr, null) catch {
+                    class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr) catch {
                         e.throwError("", "Failed to wrap native object") catch {};
                         return null;
                     };