search

mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.9k stars 639 forks source link

use mobx type for map initial data #2072

Closed BrianHung closed 9 months ago

BrianHung commented 10 months ago

What does this PR do and why?

MobX has an exported type IObservableMapInitialValues for its initialData. Since MSTMap extends ObservableMap, it makes sense that MSTMap should use the same type.

coolsoftwaretyler commented 9 months ago

So, I expected that types.map(types.string).create() would work with:

  1. [string, any][]
  2. IKeyValueMap<any>
  3. Map<string, any>
  4. undefined

It seems to only work with 2 and 4. See here.

The typings on that initialData parameter for the constructor seem to get modified somewhere between the MSTMap definition, and exposing types.map export. I don't totally see where/how.

I'm going to write a test that just tests 2/4, and I think it'll work out here. We also don't seem to allow users to call that constructor directly anyway, so I feel good about this change. I'll push a commit and merge.

BrianHung commented 9 months ago

Sorry, I've been busy with work; thanks for taking up the effort to write tests ❤️

coolsoftwaretyler commented 9 months ago

Happy to! Tryna clean up PRs so I can muck around with the folder structure lol