Merge pull request #1476 from lightpanda-io/log_unknown_object_properties

Log unknown object properties in debug builds

src/browser/js/Env.zig

@@ -142,7 +142,7 @@ pub fn init(app: *App, opts: InitOpts) !Env {
 
         const global_template_local = v8.v8__FunctionTemplate__InstanceTemplate(js_global).?;
         v8.v8__ObjectTemplate__SetNamedHandler(global_template_local, &.{
-            .getter = bridge.unknownPropertyCallback,
+            .getter = bridge.unknownWindowPropertyCallback,
             .setter = null,
             .query = null,
             .deleter = null,

src/browser/js/Snapshot.zig

@@ -261,6 +261,19 @@ pub fn create() !Snapshot {
     };
 }
 
+// Helper to check if a JsApi has a NamedIndexed handler
+fn hasNamedIndexedGetter(comptime JsApi: type) bool {
+    const declarations = @typeInfo(JsApi).@"struct".decls;
+    inline for (declarations) |d| {
+        const value = @field(JsApi, d.name);
+        const T = @TypeOf(value);
+        if (T == bridge.NamedIndexed) {
+            return true;
+        }
+    }
+    return false;
+}
+
 // Count total callbacks needed for external_references array
 fn countExternalReferences() comptime_int {
     @setEvalBranchQuota(100_000);
@@ -301,6 +314,15 @@ fn countExternalReferences() comptime_int {
         }
     }
 
+    // In debug mode, add unknown property callbacks for types without NamedIndexed
+    if (comptime IS_DEBUG) {
+        inline for (JsApis) |JsApi| {
+            if (!hasNamedIndexedGetter(JsApi)) {
+                count += 1;
+            }
+        }
+    }
+
     return count + 1; // +1 for null terminator
 }
 
@@ -357,6 +379,16 @@ fn collectExternalReferences() [countExternalReferences()]isize {
         }
     }
 
+    // In debug mode, collect unknown property callbacks for types without NamedIndexed
+    if (comptime IS_DEBUG) {
+        inline for (JsApis) |JsApi| {
+            if (!hasNamedIndexedGetter(JsApi)) {
+                references[idx] = @bitCast(@intFromPtr(bridge.unknownObjectPropertyCallback(JsApi)));
+                idx += 1;
+            }
+        }
+    }
+
     return references;
 }
 
@@ -393,6 +425,7 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio
     const instance = v8.v8__FunctionTemplate__InstanceTemplate(template);
 
     const declarations = @typeInfo(JsApi).@"struct".decls;
+    var has_named_index_getter = false;
 
     inline for (declarations) |d| {
         const name: [:0]const u8 = d.name;
@@ -453,6 +486,7 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio
                     .flags = v8.kOnlyInterceptStrings | v8.kNonMasking,
                 };
                 v8.v8__ObjectTemplate__SetNamedHandler(instance, &configuration);
+                has_named_index_getter = true;
             },
             bridge.Iterator => {
                 const function_template = @constCast(v8.v8__FunctionTemplate__New__DEFAULT2(isolate, value.func).?);
@@ -490,6 +524,23 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio
         const js_value = v8.v8__String__NewFromUtf8(isolate, JsApi.Meta.name.ptr, v8.kNormal, @intCast(JsApi.Meta.name.len));
         v8.v8__Template__Set(@ptrCast(instance), js_name, js_value, v8.ReadOnly + v8.DontDelete);
     }
+
+    if (comptime IS_DEBUG) {
+        if (!has_named_index_getter) {
+            var configuration: v8.NamedPropertyHandlerConfiguration = .{
+                .getter = bridge.unknownObjectPropertyCallback(JsApi),
+                .setter = null,
+                .query = null,
+                .deleter = null,
+                .enumerator = null,
+                .definer = null,
+                .descriptor = null,
+                .data = null,
+                .flags = v8.kOnlyInterceptStrings | v8.kNonMasking,
+            };
+            v8.v8__ObjectTemplate__SetNamedHandler(instance, &configuration);
+        }
+    }
 }
 
 fn protoIndexLookup(comptime JsApi: type) ?bridge.JsApiLookup.BackingInt {

src/browser/js/bridge.zig

@@ -410,7 +410,7 @@ const Finalizer = struct {
     from_v8: *const fn (?*const v8.WeakCallbackInfo) callconv(.c) void,
 };
 
-pub fn unknownPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 {
+pub fn unknownWindowPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 {
     const v8_isolate = v8.v8__PropertyCallbackInfo__GetIsolate(handle).?;
     var caller: Caller = undefined;
     caller.init(v8_isolate);
@@ -471,6 +471,58 @@ pub fn unknownPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.Prope
     return 0;
 }
 
+// Only used for debugging
+pub fn unknownObjectPropertyCallback(comptime JsApi: type) *const fn (?*const v8.Name, ?*const v8.PropertyCallbackInfo) callconv(.c) u8 {
+    if (comptime !IS_DEBUG) {
+        @compileError("unknownObjectPropertyCallback should only be used in debug builds");
+    }
+
+    return struct {
+        fn wrap(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 {
+            const v8_isolate = v8.v8__PropertyCallbackInfo__GetIsolate(handle).?;
+
+            var caller: Caller = undefined;
+            caller.init(v8_isolate);
+            defer caller.deinit();
+
+            const local = &caller.local;
+
+            var hs: js.HandleScope = undefined;
+            hs.init(local.isolate);
+            defer hs.deinit();
+
+            const property: []const u8 = js.String.toSlice(.{ .local = local, .handle = @ptrCast(c_name.?) }) catch {
+                return 0;
+            };
+
+            if (std.mem.startsWith(u8, property, "__")) {
+                // some frameworks will extend built-in types using a __ prefix
+                // these should always be safe to ignore.
+                return 0;
+            }
+
+            if (JsApi == @import("../webapi/cdata/Text.zig").JsApi or JsApi == @import("../webapi/cdata/Comment.zig").JsApi) {
+                if (std.mem.eql(u8, property, "tagName")) {
+                    // knockout does this, a lot.
+                    return 0;
+                }
+            }
+
+            const ignored = std.StaticStringMap(void).initComptime(.{});
+            if (!ignored.has(property)) {
+                log.debug(.unknown_prop, "unknown object property", .{
+                    .object = if (@hasDecl(JsApi.Meta, "name")) JsApi.Meta.name else @typeName(JsApi),
+                    .info = "but the property can exist in pure JS",
+                    .stack = local.stackTrace() catch "???",
+                    .property = property,
+                });
+            }
+            // not intercepted
+            return 0;
+        }
+    }.wrap;
+}
+
 // Given a Type, returns the length of the prototype chain, including self
 fn prototypeChainLength(comptime T: type) usize {
     var l: usize = 1;