Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSUtils.removeCircular is removing more stuff than it should #75

Closed
kentcdodds opened this issue Mar 13, 2015 · 3 comments
Closed

DSUtils.removeCircular is removing more stuff than it should #75

kentcdodds opened this issue Mar 13, 2015 · 3 comments
Assignees

Comments

@kentcdodds
Copy link
Contributor

Here are some examples:

screen shot 2015-03-13 at 4 47 43 pm

screen shot 2015-03-13 at 4 50 25 pm

@kentcdodds
Copy link
Contributor Author

Here's the transpiled version of that function that I'm using which results in this:

function removeCircular(object) {
  var objects = [];

  return (function rmCirc(value) {

    var i = undefined;
    var nu = undefined;

    if (typeof value === "object" && value !== null && !(value instanceof Boolean) && !(value instanceof Date) && !(value instanceof Number) && !(value instanceof RegExp) && !(value instanceof String)) {

      for (i = 0; i < objects.length; i += 1) {
        if (objects[i] === value) {
          return undefined;
        }
      }

      objects.push(value);

      if (DSUtils.isArray(value)) {
        nu = [];
        for (i = 0; i < value.length; i += 1) {
          nu[i] = rmCirc(value[i]);
        }
      } else {
        nu = {};
        forOwn(value, function (v, k) {
          return nu[k] = rmCirc(value[k]);
        });
      }
      return nu;
    }
    return value;
  })(object);
}

@jmdobry
Copy link
Member

jmdobry commented Mar 13, 2015

and the function as it was added in 1.1.0

function removeCircular(object) {
  var objects = [];

  return (function rmCirc(value) {

    var i;
    var nu;

    if (typeof value === 'object' && value !== null && !(value instanceof Boolean) && !(value instanceof Date) && !(value instanceof Number) && !(value instanceof RegExp) && !(value instanceof String)) {

      for (i = 0; i < objects.length; i += 1) {
        if (objects[i] === value) {
          return undefined;
        }
      }

      objects.push(value);

      if (DSUtils.isArray(value)) {
        nu = [];
        for (i = 0; i < value.length; i += 1) {
          nu[i] = rmCirc(value[i]);
        }
      } else {
        nu = {};
        forOwn(value, function (v, k) {
          nu[k] = rmCirc(value[k]);
        });
      }
      return nu;
    }
    return value;
  }(object));
}

@jmdobry
Copy link
Member

jmdobry commented Mar 13, 2015

I think I figured it out. It's this:

Original ES5:

forOwn(value, function (v, k) {
  nu[k] = rmCirc(value[k]);
});

New ES6: forOwn(value, (v, k) => nu[k] = rmCirc(value[k]));
Transpiled ES5:

forOwn(value, function (v, k) {
  return nu[k] = rmCirc(value[k]);
});

The forOwn function will stop iteration if false is returned. The transpiled code has a return, while the original ES5 does not. You found this because your JSON has a key with a false value. The fix is easy enough.

@jmdobry jmdobry self-assigned this Mar 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants