Support dynamic object type

[luo-occ]

it's suggested that use types.map to support dynamic object in mst, like the issue mentioned. but it puts a lot mental burden on developing. I developed a framework based on mst, and my colleague always asking me why their object shape data were converted to map. obviously, it's not accord with their intuition.

// how we consume dynamic object at present
const props = { a: 1, b: 2 };
const Model = types.map(types.number)
const instance = Model.create(props)

<Observer>
  {instance.get('a')}
  {instance.get('b')}
</Observer>

<Observer>
  {Object.fromEntries(instance)}
</Observer>

// use types.object to get a more intuitional way
const props = { a: 1, b: 2 };
const Model = types.object(types.number)
const instance = Model.create(props)

<Observer>
  {instance.a}
  {instance.b}
</Observer>

<Observer>
  {{...instance}}
</Observer>

so I propose a PR with the support of dynamic object to mst.

[jamonholmgren]

@luo-occ Thanks for the PR, and apologies for this taking so long! I've been focused on some other things.

First off, I really like this PR. Great job with it.

I'm curious what the community thinks. I think this could be a positive addition to the MST API.

What do you think about calling it types.object? I don't think this is a reserved keyword in JS or TS, and it would be more in-line with what I would expect.

Otherwise, I'm okay with this change. Would like to get more input on it from the community.

[EmilTholin]

I agree with the point that the current approach adds some initial mental burden if you are not used to working with maps, but I am not a fan of this addition.

To me this feels like a lot of extra API surface area and code for a slightly more ergonomic API (for some), that is already achievable.

[luo-occ]

@jamonholmgren Thinks for replying me, I am very glad to have your approval.

It’s also my first thought to name it types.object, but I doubt if it will cause subtle confusion between types.model and types.object, so I use types.dynamicObject to indicate it has feature of having unspecified keys (like the internal accomplishment of mobx)

Since you also prefer the name types.object, I am willing to change it.

[luo-occ]

@EmilTholin Personally, I work well with types.map. but in team collaboration, it will be challenged a lot with object shape data but map style use.

Especially when it occurs to some page builders who don’t actually use mst to build model, but just consume model, it’s drive them crazy why {id:1,name:Tom}(created by types.model) can be used like nomal object but {a:1,b:2,c:3}(created by types.map) must be used as map, it’s so confusing.

[BrianHung]

I'm curious what the community thinks. I think this could be a positive addition to the MST API.

As a newcomer to MST, I'm in support of adding types.object; surprised it wasn't already allowed as one of the basic types. I tend to work with plain javascript objects via Object.create(null), and as a result, the codebase I'm considering migrating to MST has a lot of map[key] reads and writes (and type Record from typescript) that would be tedious to convert to actual Maps instead.

Also a lot of libraries I've used in the past (e.g. immer, valtio, yjs) supported objects and arrays as primitives, and maps and sets seemed secondary.

@jamonholmgren

[BrianHung]

To add to previous point, one ability that plain objects have over maps is the ability to easily deeply destructure: https://stackoverflow.com/questions/49569682/destructuring-a-map

[coolsoftwaretyler]

Hey @luo-occ - I know it's been a while on this one, but it seems like most people are in agreement on this as a good idea.

Would you be willing to:

1. Fix the merge conflicts
2. Write a few tests specifically for this new type that exercise its API

If so, I think we can merge this in.

If you're no longer interested, let me know and I can pick that up myself and get this merged.

Thanks!

[coolsoftwaretyler]

Since this PR already has some conflicts and we want to finish up the un-monorepoization efforts, I'm going to keep this open and recommend we just rework these changes against the new structure afterwards.

[luo-occ]

I am glad to see mst has made a huge progress these days, congratulations! I will try to catch up with latest version code, fix these conflicts and add some test.

[coolsoftwaretyler]

Awesome @luo-occ. Let me know if you need anything

[constgen]

This is must have. After this nobody will use map, as map supports only string and number literals for keys. So map has no difference with object but with more complex interface.

[coolsoftwaretyler]

@constgen - yeah I would love to see this PR moved forward. We haven't heard from @luo-occ for a while. Would you want to push it forward?

No worries if not. I can always push forward as well, I just like to give folks the opportunity to contribute to things they're interested in

[constgen]

Recently discovered that Map is more optimized for frequent read-write operations than Object in JIT compilers. So Maps still make sense in some situations when speed is critical

[coolsoftwaretyler]

Hey folks, the merge conflicts on this got pretty gnarly since we moved a lot of files around, and our docs are auto-generated alongside the codebase.

I'm going to close this PR in favor of https://github.com/mobxjs/mobx-state-tree/pull/2157, where I copy/pasted @luo-occ's work into a fresh commit. I amended the git commit author to match what I saw in git log on this branch so we can give appropriate credit.

That PR is currently a draft because we need to add tests and docs. This original PR was opened before we made those contributing requirements