Remove commit and shouldCommit

[nhunzaker]

I've used Domain::commit on 3 projects, and I removed it on two of them (and I'd like to remove it on all three).

Why did we add commit?

The original motivation was to make it easier maintain one representation of data inside of a Domain where it is easy to write to, and expose a format easier to query when accessing repo.state

Why do I want to remove commit?

This worked nice for simple cases, but it has a ton of edge cases. It invites dependency on other domains in order to be useful. For example, reducing down a dataset to match a specific query.

It's also slow, potentially creating new objects where it doesn't need to. Our most recent data viz project, this was the single biggest source of inefficient rendering. It's simply too cumbersome to use shouldCommit to check to see if you should calculate state again. Often that depends on UI requirements, which don't belong in the data layer.

Also, it removes the amount of knowledge you need to know about domains. The current "special" hook methods are now: getInitialState, register, serialize, deserialize, setup, and teardown.

What to do moving forward

We have indexes and computed props for that now, so the utility is no longer there. Additionally, I want to remove as many use cases from Domains that require external knowledge as possible.

I've also removed the ImmutableJS docs for the time being. When we document indexing, we should dig into how to use them with ImmutableJS. For example:

// assume repo.state.users is an Immutable.Map()
repo.patch({ 
  users: Immutable.Map([1, { id: 1, name: 'Billy' }]) 
})

repo.index('users-list', 'users', state => Array.from(state.users))

repo.compute('users-list') // => [{ id: 1, name: "Billy" }]

repo.compute('users-list') // => Exact same users array as before

[coveralls]

Coverage increased (+0.3%) to 100.0% when pulling dd1fd4d on remove-commit into ba5c610 on master.

[greypants]

Interesting timing - I was just considering how those methods might have let me use a Vis.js DataSet as my domain state. Good to know that would have been a bad time performance wise.

🇺🇸 I'm Dan Tello and I approve of this message 🇺🇸

[nhunzaker]

@greypants oh that's a super interesting idea though. I'd love to see that.

[mackermedia]

I like 👍

[mackermedia]

🔥

[cwmanning]

👍

[coveralls]

Coverage increased (+0.3%) to 100.0% when pulling a182530 on remove-commit into 72980fd on master.

[coveralls]

Coverage remained the same at 100.0% when pulling c200903 on remove-commit into 5c0bb0c on master.