Skip to content
This repository has been archived by the owner on May 14, 2020. It is now read-only.

Support for ES6 Map #69

Closed
wants to merge 2 commits into from
Closed

Support for ES6 Map #69

wants to merge 2 commits into from

Conversation

zalmoxisus
Copy link
Collaborator

The current implementation supports only object-like maps. The problem is that we can have any data type as a map key.

So, the following example would just throw:

new Map([
  ['a', 1],
  [{toString: function(){ return 'a' }}, 2],
  [{}, 3]
])

While with this pr, it will show the following:
screen shot 2017-02-16 at 8 39 39 pm

Showing maps as an array of tuples was suggested in a productpains ticket.

@zalmoxisus
Copy link
Collaborator Author

zalmoxisus commented Feb 16, 2017

We could later also add a preview in the inspector like in Chrome DevTools:

screen shot 2017-02-16 at 9 08 12 pm

I guess here instead of indexes, we'll better use also key / value:

screen shot 2017-02-16 at 9 12 02 pm

@zalmoxisus
Copy link
Collaborator Author

zalmoxisus commented Feb 16, 2017

Alright. Now we'll show the preview as following:
screen shot 2017-02-16 at 9 16 21 pm

And expanded:
screen shot 2017-02-16 at 9 16 54 pm

I guess it's pretty good to go.

@alexkuz
Copy link
Owner

alexkuz commented Mar 5, 2017

@zalmoxisus I've implemented that in latest version, so I'm gonna close this PR.
My implementation is a bit inconsistent though - it treats string/number and object keys differently, but I think Maps rarely mix primitive and object keys together:

image

@alexkuz alexkuz closed this Mar 5, 2017
@zalmoxisus
Copy link
Collaborator Author

zalmoxisus commented Mar 5, 2017

@alexkuz thanks for implementing this!

Looks great! At first I had difficulties to get what entry 2 stands for, but agree that usually primitives aren't mixed. 👍

@alexkuz
Copy link
Owner

alexkuz commented Mar 5, 2017

yes, there will be also an update for inspector soon.

@alexkuz
Copy link
Owner

alexkuz commented Mar 5, 2017

@zalmoxisus you can try v0.11.3 now.

@zalmoxisus
Copy link
Collaborator Author

@alexkuz awesome! Thanks!

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

Successfully merging this pull request may close these issues.

2 participants