From bba4f8e6b8da534b7c4673bc5e7fe7191a6f93e6 Mon Sep 17 00:00:00 2001
From: Pierre Le Fevre <hello@pierrelf.com>
Date: Fri, 27 Mar 2026 20:56:50 +0100
Subject: [PATCH] Review fixes: remove dead code, fix misplaced doc comment

- Remove unused `val_str` variable in `handle_style_set`
- Remove `let _ = bridge;` dead code suppression in `create_class_list`,
  prefix param with underscore instead
- Move `collect_roots` doc comment to correct method (was accidentally
  attached to `resolve_dom_property`)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 crates/js/src/dom_bridge.rs | 8 +-------
 crates/js/src/vm.rs         | 2 +-
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/crates/js/src/dom_bridge.rs b/crates/js/src/dom_bridge.rs
index 366e02f..7e471b9 100644
--- a/crates/js/src/dom_bridge.rs
+++ b/crates/js/src/dom_bridge.rs
@@ -1254,7 +1254,6 @@ pub fn handle_style_set(
     };
 
     // Set the property on the style object normally.
-    let val_str = val.to_js_string(gc);
     if let Some(HeapObject::Object(data)) = gc.get_mut(gc_ref) {
         data.properties
             .insert(key.to_string(), Property::data(val.clone()));
@@ -1267,14 +1266,12 @@ pub fn handle_style_set(
         .borrow_mut()
         .set_attribute(node_id, "style", &style_str);
 
-    // Also set it as a normal property (already done above), but tell caller it was handled.
-    let _ = val_str;
     true
 }
 
 // ── classList helpers ───────────────────────────────────────────────
 
-fn create_class_list(gc: &mut Gc<HeapObject>, bridge: &DomBridge, node_id: NodeId) -> Value {
+fn create_class_list(gc: &mut Gc<HeapObject>, _bridge: &DomBridge, node_id: NodeId) -> Value {
     let mut data = ObjectData::new();
     // Store the node ID for method callbacks.
     data.properties.insert(
@@ -1295,9 +1292,6 @@ fn create_class_list(gc: &mut Gc<HeapObject>, bridge: &DomBridge, node_id: NodeI
         set_builtin_prop(gc, gc_ref, name, Value::Function(func));
     }
 
-    // Also keep a reference to the bridge node_wrappers to avoid issues.
-    // The classList methods use ctx.dom_bridge to access the document.
-    let _ = bridge;
     Value::Object(gc_ref)
 }
 
diff --git a/crates/js/src/vm.rs b/crates/js/src/vm.rs
index 4e94434..b8f0b32 100644
--- a/crates/js/src/vm.rs
+++ b/crates/js/src/vm.rs
@@ -1958,7 +1958,6 @@ impl Vm {
         self.gc.alloc(HeapObject::Object(obj))
     }
 
-    /// Collect all GcRef values reachable from the mutator (roots for GC).
     /// Resolve a dynamic DOM property for a wrapper object.
     /// Returns `Some(value)` if the key is a recognized DOM property, `None` otherwise.
     fn resolve_dom_property(&mut self, gc_ref: GcRef, key: &str) -> Option<Value> {
@@ -1981,6 +1980,7 @@ impl Vm {
         }
     }
 
+    /// Collect all GcRef values reachable from the mutator (roots for GC).
     fn collect_roots(&self) -> Vec<GcRef> {
         let mut roots = Vec::new();
         for val in &self.registers {
-- 
2.51.2