From 7cf0ab40e541db66ba90235530468af9290d5ce4 Mon Sep 17 00:00:00 2001
From: Josh Howenstine <joshhowenstine@gmail.com>
Date: Tue, 3 Dec 2024 11:59:28 -0800
Subject: [PATCH] fix: createSharedReferences not comparing values (#549)

* fix: check for values in shared references

* fix: code cleanup

* fix: refactor createSharedReferences

* fix: formatting updates

* fix: remove test file

* fix: remove comments in test

* fix: formatting issues
---
 .../src/mixins/withThemeStyles/utils.js       | 56 +++++++++++--------
 .../src/mixins/withThemeStyles/utils.test.js  | 28 ++++++++++
 2 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js
index 0d77d991d..47116b2ec 100644
--- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js
+++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.js
@@ -209,31 +209,44 @@ export function removeEmptyObjects(obj) {
   return obj; // Always return obj, even if it's empty
 }
 
-// This map will store hashes of objects to detect duplicates.
+export function safeStringify(originalObj) {
+  const obj = { ...originalObj };
 
-export function createSharedReferences(obj = {}) {
-  const seenObjects = new Map();
+  const seen = new WeakSet(); // WeakSet is used to store references to objects we've processed
 
-  // Generates a hash for an object.
-  // Sorting keys ensures consistent hash regardless of property order.
-  function hash(object) {
-    return JSON.stringify(object, Object.keys(object).sort());
-  }
+  return JSON.stringify(obj, (key, value) => {
+    if (typeof value === 'object' && value !== null) {
+      if (seen.has(value)) {
+        return '[Circular]'; // Replace circular references with a string
+      }
+      seen.add(value); // Mark this object as seen
+    }
+    return value; // Return the value as is
+  });
+}
+
+export function createSharedReferences(obj = {}) {
+  const seenObjects = new Map(); // Store original reference -> shared reference
 
   function process(currentObj) {
-    for (const key in currentObj) {
-      if (currentObj.hasOwnProperty(key)) {
-        const value = currentObj[key];
-        if (typeof value === 'object' && value !== null) {
-          // Ensure it's an object
-          const valueHash = hash(value);
-          if (seenObjects.has(valueHash)) {
-            // If we've seen this object before, replace the current reference
-            // with the original reference.
-            currentObj[key] = seenObjects.get(valueHash);
-          } else {
-            seenObjects.set(valueHash, value);
-            process(value); // Recursively process this object
+    const queue = [currentObj]; // Use a queue for breadth-first traversal
+
+    while (queue.length > 0) {
+      const current = queue.shift();
+
+      for (const key in current) {
+        if (current.hasOwnProperty(key)) {
+          const value = current[key];
+          if (typeof value === 'object' && value !== null) {
+            const cacheKey = safeStringify(value);
+            if (seenObjects.has(cacheKey)) {
+              // Replace duplicate reference with the shared reference
+              current[key] = seenObjects.get(cacheKey);
+            } else {
+              // Add child objects to the queue for processing
+              seenObjects.set(cacheKey, value);
+              queue.push(value);
+            }
           }
         }
       }
@@ -241,7 +254,6 @@ export function createSharedReferences(obj = {}) {
   }
 
   process(obj);
-
   return obj;
 }
 
diff --git a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js
index a6bb7e67a..5fdfb65b5 100644
--- a/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js
+++ b/packages/@lightningjs/ui-components/src/mixins/withThemeStyles/utils.test.js
@@ -452,6 +452,34 @@ describe('createSharedReferences', () => {
     const result = createSharedReferences(input);
     expect(result.a).not.toBe(result.b);
   });
+
+  it('should preserve circular references', () => {
+    const obj = {};
+    obj.self = obj;
+    const result = createSharedReferences(obj);
+    expect(result.self).toBe(result);
+  });
+
+  it('should preserve nested circular references', () => {
+    const obj = { a: {} };
+    obj.a.self = obj.a;
+    const result = createSharedReferences(obj);
+    expect(result.a.self).toBe(result.a);
+  });
+
+  it('should preserve shared references for the same object', () => {
+    const shared = {};
+    const obj = { a: shared, b: shared };
+    const result = createSharedReferences(obj);
+    expect(result.a).toBe(result.b);
+  });
+
+  it('should preserve deeply nested circular references', () => {
+    const obj = { a: { b: { c: { d: {} } } } };
+    obj.a.b.c.d.self = obj.a.b.c.d;
+    const result = createSharedReferences(obj);
+    expect(result.a.b.c.d.self).toBe(result.a.b.c.d);
+  });
 });
 
 describe('getUniqueProperties', () => {