Handle circular reference with toJS

Fixes #1789
immutable-js · Jul 19, 2021 · 0e8272e · 0e8272e
1 parent e80d8fa
commit 0e8272e
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 5 deletions.
diff --git a/__tests__/Conversion.ts b/__tests__/Conversion.ts
@@ -1,4 +1,11 @@
-import { fromJS, is, List, Map, OrderedMap, Record } from 'immutable';
+import {
+  fromJS,
+  is,
+  List,
+  Map,
+  OrderedMap,
+  Record as ImmutableRecord,
+} from 'immutable';
 
 import * as jasmineCheck from 'jasmine-check';
 jasmineCheck.install();
@@ -30,7 +37,7 @@ describe('Conversion', () => {
     list: [1, 2, 3],
   };
 
-  const Point = Record({ x: 0, y: 0 }, 'Point');
+  const Point = ImmutableRecord({ x: 0, y: 0 }, 'Point');
 
   const immutableData = Map({
     deepList: List.of(
@@ -114,6 +121,27 @@ describe('Conversion', () => {
     );
   });
 
+  it('Throws when calling toJS with a circular reference', () => {
+    const obj: Record<string, any> = {
+      foo: 1,
+      bar: null,
+    };
+    // Create circular reference
+    obj.bar = obj;
+
+    const simplerTest = List().push(obj);
+
+    expect(() => simplerTest.toJS()).toThrow(
+      'Cannot convert circular structure to JS'
+    );
+  });
+
+  it('does not throw when using empty objects', () => {
+    expect(() => fromJS([{}, {}]).toJS()).not.toThrow();
+    expect(() => fromJS([['', '']]).toJS()).not.toThrow();
+    expect(() => fromJS([[null, null]]).toJS()).not.toThrow();
+  });
+
   it('Converts deep JSON with custom conversion', () => {
     const seq = fromJS(js, function (key, sequence) {
       if (key === 'point') {
@@ -173,7 +201,7 @@ describe('Conversion', () => {
   });
 
   it('JSON.stringify() respects toJSON methods on values', () => {
-    const Model = Record({});
+    const Model = ImmutableRecord({});
     Model.prototype.toJSON = function () {
       return 'model';
     };

diff --git a/src/toJS.js b/src/toJS.js
@@ -2,8 +2,30 @@ import { Seq } from './Seq';
 import { isCollection } from './predicates/isCollection';
 import { isKeyed } from './predicates/isKeyed';
 import isDataStructure from './utils/isDataStructure';
+import isPlainObject from './utils/isPlainObj';
 
 export function toJS(value) {
+  const circularStack =
+    typeof WeakSet === 'undefined' ? new Set() : new WeakSet(); // WeakSet is not available in IE11.
+
+  return toJSWithCircularCheck(circularStack, value);
+}
+
+function checkCircular(circularStack, value) {
+  if (!isPlainObject(value)) {
+    return;
+  }
+
+  if (circularStack.has(value)) {
+    throw new TypeError('Cannot convert circular structure to JS');
+  }
+
+  circularStack.add(value);
+}
+
+function toJSWithCircularCheck(circularStack, value) {
+  checkCircular(circularStack, value);
+
   if (!value || typeof value !== 'object') {
     return value;
   }
@@ -16,13 +38,15 @@ export function toJS(value) {
   if (isKeyed(value)) {
     const result = {};
     value.__iterate((v, k) => {
-      result[k] = toJS(v);
+      result[k] = toJSWithCircularCheck(circularStack, v);
     });
     return result;
   }
   const result = [];
   value.__iterate(v => {
-    result.push(toJS(v));
+    result.push(toJSWithCircularCheck(circularStack, v));
   });
+  circularStack.delete(value);
+
   return result;
 }