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

Add support for Map nodes #66

Closed
wants to merge 1 commit into from
Closed

Add support for Map nodes #66

wants to merge 1 commit into from

Conversation

bnoon
Copy link

@bnoon bnoon commented Aug 13, 2015

I use Map() in my state for ordered lists and would like to inspect them.

@danielkcz
Copy link
Contributor

I think that this should be sorted out in a more generic way, since there is a lack of support for ImmutableJS too (see #51). The Map from ES6 has method toJSON same as the ImmutableJS objects. If we use duck typing for such method, we don't need additional node type in my opinion.

Edit: I am sorry, there is actually just toJS method on Immutable objects. So it would require double check, but it still feels much easier than copy&pasting and changing node.

@bnoon
Copy link
Author

bnoon commented Aug 13, 2015

I agree that the copy/paste was a quick hack and the common functionality should be pulled out.

However, Map is a built-in object and sufficiently different from Object or Array to be special cased.

@danielkcz
Copy link
Contributor

Yeah, I guess that Map is different, but it's pretty much same as Map from Immutable. There is problem that you can have basically anything in your key, so it cannot be displayed easily in a tree view. Map can be more likely seen as a list of entries with keys and values.

@gaearon
Copy link
Contributor

gaearon commented Aug 13, 2015

Can we somehow use iterators + entries() method to have a generic way to enumerate stuff?

@danielkcz
Copy link
Contributor

@gaearon Yeah, I think that might be a good way. The result is in both cases identical (https://jsbin.com/pirura/edit?js,console). And actually entries() is default iterator. It might be enough to check if there is an iterator present obj[Symbol.iterator].

@bnoon
Copy link
Author

bnoon commented Aug 21, 2015

I am closing this in favor of #79. Thanks @FredyC!

@bnoon bnoon closed this Aug 21, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 23, 2015

Should be fixed in 1.1.0, please verify.

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

Successfully merging this pull request may close these issues.

3 participants