feat!(use-map-state): Actually uses a `Map`

[ChocolateLoverRaj]

Warning - breaking changes

Added type parameters to the function.

Used useState with the controls object instead of individual useCallbacks.

Removed has control because it is readonly an Map.prototype.has() can be used.

Updated tests to use Map.

Removed unnecessary checks in the tests.

Updated docs to use new args and return type of hook.

Added pnpm-lock.yaml because I used pnpm i.

[imbhargav5]

@ChocolateLoverRaj Pretty solid but I have a few concerns.

• I don't want to introduce breaking changes at this point.
• I don't want to introduce pnpm changes or unrelated typescript changes in this PR as that is a different topic.

But your idea is really good as this allows us to use Map. I personally want useMapState to be usable with objects too as they are pretty map-like.

So how do you feel about making this hook work for both objects and maps? We can do a call and work on this together if you want to.

I want to really make sure your changes go into main as I like your passion and this idea is also pretty good.

[ChocolateLoverRaj]

@imbhargav5 Based on your feedback I will do the following things:

• Remove pnpm-lock.yaml and add it to .gitignore
• Put the typescript change in a seperate commit? or PR? The downleveliteration is not unrelated because it is needed to rest spread a map like [...map].
• Keep backwards compatibility by using the old hook if an object is given initially, and use the new hook if a map is given initially. It might get confusing though because then the hook will have two different overloads. It would make sense if the current hook was renamed to useObjectState or something other than useMapState. I think we should do both, and have a useObjectState and a useMapState. For backwards compatibility, the useMapState will work with objects, but for clarity, people can start using useObjectState when writing new code. Then maybe if there are breaking changes in the future, useMapState can deprecate the feature of working with maps.

[imbhargav5]

Hmm. Yeah. The overloads will be confusing but I think useMapState should be able to work with Maps and Objects since they are pretty similar in Javascript. That is just a preference but we can take a couple of days to think about it. I want to make sure we make the right call.

[ChocolateLoverRaj]

@imbhargav5 One improvement I made to the Map version is that instead of returning [map, controls], it just returns map, and the map methods which modify the map use setState instead. This makes it more convenient.

[ChocolateLoverRaj]

@imbhargav5 Can you review https://github.com/imbhargav5/rooks/pull/531/commits/9fd1c8223de0f7e1064a14d26f8a2c150f7dedec?

[ChocolateLoverRaj]

For now I'm using rooks by using github:ChocolateLoverRaj/rooks#build/preview-531 as my rooks version.

[imbhargav5]

@qiweiii please take a look at this one.

[qiweiii]

@ChocolateLoverRaj, https://github.com/imbhargav5/rooks/pull/531/commits/9fd1c8223de0f7e1064a14d26f8a2c150f7dedec looks great to me, the only thing is that it only accepts Map type as initalValue.

I was thinking how to make it work with both Object and Map, and also make it more backwards compatible, so a solution I want to suggest is this:

• Check if the initialValue is an object literal or an object created with new Object(), using Object.getPrototypeOf(obj) === Object.prototype
• If it is, we can use new Map(Object.enties(initialValue)) to turn it into a Map first

reference:

• https://stackoverflow.com/a/43773857/9055088
• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries#converting_an_object_to_a_map

[qiweiii]

@ChocolateLoverRaj just need to resolve the conflict first, and I think it's good to merge, @imbhargav5

[imbhargav5]

Got it @qiweiii . I will review this today or tomorrow.

[codecov[bot]]

Codecov Report

Merging #531 (f345019) into main (a50dd21) will increase coverage by 0.20%. The diff coverage is 80.95%.

@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
+ Coverage   72.93%   73.14%   +0.20%     
==========================================
  Files          66       69       +3     
  Lines        1319     1359      +40     
  Branches      265      267       +2     
==========================================
+ Hits          962      994      +32     
- Misses        356      364       +8     
  Partials        1        1              

Impacted Files	Coverage Δ
src/hooks/useMapState/useMapState.ts	75.00% <75.00%> (ø)
src/hooks/useMapState/useMapOrObjectState.ts	90.90% <90.90%> (ø)
src/hooks/useMapState/index.ts	100.00% <100.00%> (ø)
src/hooks/useMapState/useObjectState.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a50dd21...f345019. Read the comment docs.

[imbhargav5]

New work for this PR in #1284