[Feature] map type: infer keys from identifiers on snapshot

[jlgrall]

Description of feature:

map.put() already infers the key from the identifier

Add the same behavior for when we apply a new snapshot, so that we can pass a list of unkeyed elements, and their keys will be automatically inferred from the identifiers.

How:

If the map receives an array instead of a javascript object, it will automatically discard the keys and infer them from the identifier. Note: this should only apply when there is an identifier in the element model.

Reasons:

• Avoids the necessity of generating the temporary keyed object map when the server sends an unkeyed array of elements.
• Avoids undefined behavior or possible errors in case where the key doesn't match the identifier (for example: a human written config).

Example:

const Todo = types.model({
  id: types.identifier,
  task: types.string
})

const TodoStore = types.model({
  todos: types.map(Todo)
})

const s = TodoStore.create({
  todos: [    // Automatically infers the map keys from the identifiers
    { task: "Take the bus", id: 20 },
    { task: "Buy milk", id: 5 }
  ]
})
console.log(s.todos.get(5).task) // prints: "Buy milk"

[xaviergonz]

well, the weird thing about this is that the input snapshot (and maybe the output snapshot?) would be different from the intended type, which would be a map.

to me it sounds more like you actually want a types.array(some model) where the instance array type has a new method called findById(id)

this is, something like:

const Todo = types.model({
  id: types.identifier,
  task: types.string
})

const TodoStore = types.model({
  todos: types.array(Todo) // array instead of map
})

const s = TodoStore.create({
  todos: [
    { task: "Take the bus", id: 20 },
    { task: "Buy milk", id: 5 }
  ]
})
console.log(s.todos.findById(5).task) // prints: "Buy milk", note findById is used

would that be ok? that way the input and output snapshots would be consistent with the actual type (an array)

although basically that'd be the same as

s.todos.find(i => getIdentifier(i) === 5).task

// or more generic
function findById(array, id) { return array.find(i => getIdentifier(i) === id)  }
findById(s.todos, 5).task

[xaviergonz]

@mweststrate would it make sense to add a resolve/get/findById/whatever method to the instance arrays for this use case?

my worry is how it should behave with arrays of non models / composed models / unions

[jlgrall]

Yes, it would work with using an array instead of a map and adding a resolve/get/findById/whatever.

But I am not sure that it would make sense, because I never use it as an array: I never loop, map/reduce or filter over the elements. I don't even access or care about its size. Instead, I always access, add or remove a specific element through a known identifier (which is usually a string). Sorry if the example in the first post doesn't show this, I took it from the doc, and it is too short to show a real situation.

You gave this snippet as an example of how it should be implemented: s.todos.find(i => getIdentifier(i) === 5).task Sorry, but this is probably the worst way of doing it. It accesses every elements until it finds the right one, so mobx will record a dependency between all these unrelated elements that are accessed (even though they are not used), and the current computed value or reaction. You just added an exponential number of unneeded dependencies between unrelated elements: if any of these elements changes, many unrelated computed values or reactions will now need to be recomputed for nothing.

[mweststrate]

@xaviergonz I was thinking myself a while back to give types.map a second argument with options, to define the serialization scheme:

types.map(SomeType, { serializeAs: "object" | "entries" | "array" })

One of the reasons is that we now always do "object" style, which is fine as default, but the best practices is to use an entry of arrays, e.g.

"object"
{
    K1: V1,
    K2: V2
}

"entries"
[
   [K1, V1],
   [K2, V2]
]

"array"
[V1, V2] // (and infer identifiers) 

The reason why the standard JS map uses the entries approach, is that it is more flexible in keys. Objects cannot store keys numeric keys (they will always be stringified), however, in the entries format it is actually possible to store all weird things as keys (even objects! but that is not applicable to MST)

[xaviergonz]

sounds good to me!

[jrmyio]

I am running into an issue related to this. ES6 Maps are meant to keep order:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map "The Map object holds key-value pairs and remembers the original insertion order of the keys."

However, when creating a snapshot an object is created. That object will not remain the order (especially if you have numeric keys). In this case, serializing the MST object with a map in localstorage will bring a different order once you load the object from localstorage (if your keys don't match your map's order).

The entries or array serializion would not suffer from this problem as the order would be kept.

Considering maps are created with an array of [key, value] definitions:

Map([
  [1, 'one'],
  [2, 'two'],
  [3, 'three'],
])

I think defining snapshots of maps using this way would have actually been a better default.