Preserve referential equality for rules

Author: devongovett

node/src/rule_list.rs

@@ -1,4 +1,4 @@
-use napi::{CallContext, JsNumber, JsObject, JsUndefined, JsUnknown, JsFunction, Property, Ref, Result, Env};
+use napi::{CallContext, sys, JsNumber, JsObject, NapiRaw, NapiValue, JsUndefined, JsUnknown, JsFunction, Property, Ref, Result, Env};
 use std::cell::RefCell;
 use parcel_css::stylesheet::StyleSheet;
 use std::rc::Rc;
@@ -7,6 +7,11 @@ use crate::rule;
 // TODO: this is probably not safe.
 static mut CLASS: RefCell<Option<Ref<()>>> = RefCell::new(None);
 
+struct CSSRuleList {
+  stylesheet: Rc<RefCell<StyleSheet>>,
+  rules: Vec<sys::napi_ref>
+}
+
 #[js_function(0)]
 fn constructor(ctx: CallContext) -> Result<JsUndefined> {
   ctx.env.get_undefined()
@@ -15,17 +20,42 @@ fn constructor(ctx: CallContext) -> Result<JsUndefined> {
 #[js_function(0)]
 fn get_length(ctx: CallContext) -> Result<JsNumber> {
   let this: JsObject = ctx.this_unchecked();
-  let stylesheet: &mut Rc<RefCell<StyleSheet>> = ctx.env.unwrap(&this)?;
-  ctx.env.create_uint32(stylesheet.borrow().rules.0.len() as u32)
+  let list: &mut CSSRuleList = ctx.env.unwrap(&this)?;
+  ctx.env.create_uint32(list.stylesheet.borrow().rules.0.len() as u32)
 }
 
 #[js_function(1)]
 fn item(ctx: CallContext) -> Result<JsObject> {
   let this: JsObject = ctx.this_unchecked();
-  let index = ctx.get::<JsNumber>(0)?.get_uint32()?;
-  let stylesheet: &mut Rc<RefCell<StyleSheet>> = ctx.env.unwrap(&this)?;
-  let r = stylesheet.borrow().rules.0[index as usize].clone();
-  rule::create(ctx.env, r)
+  let index = ctx.get::<JsNumber>(0)?.get_uint32()? as usize;
+  let list: &mut CSSRuleList = ctx.env.unwrap(&this)?;
+
+  // See if we already have a JS object created for this rule to preserve referential equality.
+  // e.g. rules.item(0) === rules.item(0)
+  // This drops down to low level C bindings for napi because there is currently no way
+  // to create a weak reference in napi-rs.
+  match &list.rules.get(index) {
+    Some(r) if !r.is_null() => {
+      let mut js_value = std::ptr::null_mut();
+      unsafe {
+        sys::napi_get_reference_value(ctx.env.raw(), **r, &mut js_value);
+        if !js_value.is_null() {
+          return JsObject::from_raw(ctx.env.raw(), js_value)
+        }
+      };
+    }
+    _ => {}
+  };
+
+  let r = list.stylesheet.borrow().rules.0[index].clone();
+  let obj = rule::create(ctx.env, r)?;
+  while list.rules.len() <= index {
+    list.rules.push(std::ptr::null_mut());
+  }
+  unsafe {
+    sys::napi_create_reference(ctx.env.raw(), obj.raw(), 0, &mut list.rules[index]);
+  };
+  Ok(obj)
 }
 
 pub fn init(exports: &mut JsObject, env: Env) -> Result<()> {
@@ -41,11 +71,11 @@ pub fn init(exports: &mut JsObject, env: Env) -> Result<()> {
   Ok(())
 }
 
-pub fn create(env: &Env, list: Rc<RefCell<StyleSheet>>) -> Result<JsObject> {
+pub fn create(env: &Env, stylesheet: Rc<RefCell<StyleSheet>>) -> Result<JsObject> {
   let r = unsafe { CLASS.borrow() };
   let r = r.as_ref().unwrap();
   let c: JsFunction = env.get_reference_value(&r)?;
   let mut instance = c.new::<JsUnknown>(&[])?;
-  env.wrap(&mut instance, list)?;
+  env.wrap(&mut instance, CSSRuleList { stylesheet, rules: Vec::new() })?;
   Ok(instance)
 }

test.js

@@ -19,6 +19,7 @@ stylesheet.replaceSync(`
 
 console.log(rules.length)
 console.log(rules.item(0) instanceof CSSStyleRule, rules.item(0) instanceof CSSRule);
+console.log(rules.item(0) === rules.item(0));
 console.log(rules.item(0).cssText);
 console.log(rules.item(0).selectorText);