skortchmark9 / reselect-tools

Debugging Tools for Reselect
MIT License
426 stars 24 forks source link

Support Immutable values #28

Closed XuluWarrior closed 5 years ago

XuluWarrior commented 6 years ago

Currently, if a selector outputs Immutable values these are shown in the state tree in their rawest object form e.g. Immutable.Map({a:1}) becomes

{
  size : 1
 __altered : false
 _root:
  entries : [
   "a"
  ]
  ownerID: {}
}

rather than

{
  a : 1
}
skortchmark9 commented 6 years ago

Hey @XuluWarrior thanks for the issue / PR - I don't think I have notifications set up properly for this repository.

My feeling right now is that I'd prefer selectorGraph() to output pure a JSON dict (or string, if we need to) Any reason you decided to make the change in the extension rather than in the lib?

XuluWarrior commented 6 years ago

I actually originally made changes in the lib but then thought that perhaps the intention of selectorGraph was to return the actual selector values rather than reflections of them. It would then be up to the consumer of the graph as to how to use/format it.

As my only current use of the lib is to show the results in the extension, I don't mind where the change is.

skortchmark9 commented 6 years ago

Yeah I think I'm sold on having it here.

anikolaev commented 5 years ago

Is it implemented? I still see internal structure of Immutable objects in Reselect Devtools Exension@0.0.2 + reselect-tools@^0.0.7.

XuluWarrior commented 5 years ago

It doesn't look like it. My PR hasn't been merged yet.

anikolaev commented 5 years ago

@XuluWarrior wasn't it decided to have this been fixed in the extension?

XuluWarrior commented 5 years ago

@anikolaev my PR (#4) is for the extension

anikolaev commented 5 years ago

@XuluWarrior Oh, I see now. I first thought you are the owner of the extension repo.

anikolaev commented 5 years ago

@skortchmark9 could we have this merged?

skortchmark9 commented 5 years ago

Merged now. Sorry about the delay I've been doing other things.

skortchmark9 commented 5 years ago

Moving all extension issues into the main repo, per #24