util: use proper circular reference checking

addaleax · MylesBorins · commit c3c6cb1c1388 · 2017-09-12T03:16:59.000+02:00
Circular references are conceptually nothing that should be checked for objects (or Sets or Maps) only, but applies to recursive structures in general, so move the `seen` checks into a position where it is part of the recursion itself. Fixes: #14758 PR-URL: #14790 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
diff --git a/lib/util.js b/lib/util.js
@@ -612,10 +612,13 @@ function formatValue(ctx, value, recurseTimes) {
     }
   }
 
-  ctx.seen.push(value);
-
-  var output = formatter(ctx, value, recurseTimes, visibleKeys, keys);
+  // TODO(addaleax): Make `seen` a Set to avoid linear-time lookup.
+  if (ctx.seen.includes(value)) {
+    return ctx.stylize('[Circular]', 'special');
+  }
 
+  ctx.seen.push(value);
+  const output = formatter(ctx, value, recurseTimes, visibleKeys, keys);
   ctx.seen.pop();
 
   return reduceToSingleString(output, base, braces, ctx.breakLength);
@@ -835,21 +838,17 @@ function formatProperty(ctx, value, recurseTimes, visibleKeys, key, array) {
     }
   }
   if (!str) {
-    if (ctx.seen.indexOf(desc.value) < 0) {
-      if (recurseTimes === null) {
-        str = formatValue(ctx, desc.value, null);
+    if (recurseTimes === null) {
+      str = formatValue(ctx, desc.value, null);
+    } else {
+      str = formatValue(ctx, desc.value, recurseTimes - 1);
+    }
+    if (str.indexOf('\n') > -1) {
+      if (array) {
+        str = str.replace(/\n/g, '\n  ');
       } else {
-        str = formatValue(ctx, desc.value, recurseTimes - 1);
-      }
-      if (str.indexOf('\n') > -1) {
-        if (array) {
-          str = str.replace(/\n/g, '\n  ');
-        } else {
-          str = str.replace(/^|\n/g, '\n   ');
-        }
+        str = str.replace(/^|\n/g, '\n   ');
       }
-    } else {
-      str = ctx.stylize('[Circular]', 'special');
     }
   }
   if (name === undefined) {
