mobxjs / mobx-state-tree

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

Fix(docs): change mobx values call to native map values call in Getting Started tutorial #1982

Closed alexamy closed 1 year ago

alexamy commented 1 year ago

Follow up for https://github.com/mobxjs/mobx-state-tree/discussions/1981.

Problem

There is a typing error when using Mobx utility values on MST map collection. You can see it in this playground (for todo.name on line 23).

Solution

Use native Map method values with Array.from:

// from this
values(store.todos);
// to this
Array.from(store.todos.values());

Other updates

EmilTholin commented 1 year ago

I'm thinking it might be worth changing the todos in the tutorial from a map to an array?

This way we will use a map for the Users and an array for the todos showcasing both data types, and we don't have to use values or Array.from(todos.values()) in the introduction, which might be a bit more of a complex use case.

Just some food for thought, @jamonholmgren will have better input on this than me.

jamonholmgren commented 1 year ago

I agree it would make more sense to use an array there. Either of you want to tackle that? 😀

alexamy commented 1 year ago

I'll try to do that and make new typescript playgrounds by the way.

EmilTholin commented 1 year ago

Making sure the code in the Getting Started tutorial doesn't cause trouble when ported to TypeScript is one thing, but I think we still want to keep it in JavaScript.

alexamy commented 1 year ago

Ok, I'll revert all typescript related changes.

alexamy commented 1 year ago

@EmilTholin I've reverted changes to just replacing values from mobx to native Array.from / values call. I think other changes (convert todos from object to array) can be done in separate PR.