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

Create Map from Object which has length Property #1026

Closed
jaganathanb opened this issue Dec 20, 2016 · 4 comments
Closed

Create Map from Object which has length Property #1026

jaganathanb opened this issue Dec 20, 2016 · 4 comments

Comments

@jaganathanb
Copy link

jaganathanb commented Dec 20, 2016

There is a question in SO which talks about custom fromJS

fromJS is taken from the Immutable docs page

function fromJS(js) {
    return typeof js !== 'object' || js === null ? js :
        Array.isArray(js) ? 
            Immutable.Seq(js).map(fromJS).toOrderedSet() :
            Immutable.Seq(js).map(fromJS).toMap();
}

var output = fromJS({
    measurements: {
        length: 5,
        weight: 30
    }
}).toJS();

The above code outputs,

//output
[object Object] {
  measurements: [object Object] {
    0: undefined,
    1: undefined,
    2: undefined,
    3: undefined,
    4: undefined
  }
}

But it has to be printed as below.

[object Object] {
  measurements: [object Object] {
    length: 5,
    weight: 30
  }
}

What is wrong with the above custom fromJS function.

JSBin is here

Note: If we change the property length to mLength, it works as expected.

@kitten
Copy link

kitten commented Jan 3, 2017

@jaganathanb The problem seems to be here: https://github.com/facebook/immutable-js/blob/master/src/Seq.js#L359

Seq is creating an ArraySeq if the object passed has a property length that is a number, which seems to be bollocks. I'm not sure why it isn't using Array.isArray.

You could just write a version that doesn't use Seq for plain objects:

function fromJS(x) {
  if (Array.isArray(x)) {
    return new List(x).map(fromJS)
  } else if (typeof x === 'object') {
    return Object.keys(x).reduce((acc, key) => (
      acc.set(key, fromJS(x[key]))
    ), new Map().asMutable())
  }

  return x
}

This is of course not optimal, but it should work.

@ShirtlessKirk
Copy link

ShirtlessKirk commented Jan 9, 2017

@philpl "I'm not sure why it isn't using Array.isArray"

Because things like NodeLists and arguments aren't arrays, but are array-like. The issue here is that if there is a length property, to truly check if it is array-like the key of length - 1 should be defined on the object:

var o = {
  '4': undefined, // array 'index' property, set to undefined 
  // to muck with simple checks like a[a.length - 1] != undefined. Muhaha.
  length: 5,
  width: 30
}; // o.hasOwnProperty(o.length - 1) === true, so object is truly array-like

@kitten
Copy link

kitten commented Jan 10, 2017

@ShirtlessKirk True! I didn't think of those. In that case, I'll open a PR for the extra check.

@leebyron
Copy link
Collaborator

leebyron commented Mar 3, 2017

In the meantime, Seq.Keyed() will not expect array-likes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants