search

yury-dymov / json-api-normalizer

Normalize JSON API data for redux applications
MIT License
576 stars 40 forks source link

WIP: change plain objects to Maps #13

Closed JaKXz closed 7 years ago

JaKXz commented 7 years ago

@yury-dymov thanks for this library, it's pretty great :)

I'm just curious what you think of this change. It's pretty simple: instead of plain objects, I suggest using JS Maps, because they provide many features and improvements... I am open to feedback & looking forward to your thoughts.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.6%) to 98.75% when pulling 4dc4fca1cab85bfe9989927174e44ce9b9c97db9 on JaKXz:feat/use-map into a531f36a99c7fa8b32c2eaa6191e121771b4e6cc on yury-dymov:master.

yury-dymov commented 7 years ago

Hi @JaKXz, thank you for contributing.

To be honest, I don't see much value in this change.

IE8 doesn't support Maps, which is not an issue for me but as a developer I would rather expect to get an Object, rather than Map from API. I assume that many folks are also using some kind of Immutable stuff for many good reasons and they are expecting to get an Object.

Also, library is doing very simple thing: converting JSON API document to the redux-friendly storage formant. As we are almost always middleware, I doubt that library consumer will get any benefits of using Maps over JS Objects.

Meanwhile, I will leave this open for some time. Maybe, there would be other opinions or use-cases, which didn't come to my mind.