fuzetsu / mergerino

immutable merge util for state management
MIT License
71 stars 6 forks source link

Handling of primitive values as the first argument #11

Open EvgenyOrekhov opened 4 years ago

EvgenyOrekhov commented 4 years ago

Hey there! Really cool util! I was pretty satisfied with Ramda, but mergerino combines several Ramda functions into one and then some. It reduces boilerplate code even more. Thank you for this util!

I'm considering integrating into my framework-agnostic state management library - actus (it's in early alpha stage).

I can see an edge case where mergerino could be improved. It's when the first argument is a primitive value.

These cases work as expected:

merge({a: 1}, {a: () => 2}); // {a: 2}
merge(1, () => 2); // 2

But these work inconsistently:

merge({a: 1}, {a: (previous) => previous + 1}); // {a: 2} (as expected)
merge([1], (previous) => [...previous, 2]); // [1, 2] (as expected)

merge(1, (previous) => previous + 1); // "[object Object]1" (expected to see 2)
merge({a: 1}, {a: 2}); // {a: 2} (as expected)

merge(1, 2); // {} (expected to see 2)

Same applies to other primitive types, like strings and booleans:

merge({a: false}, {a: (previous) => !previous}); // {a: true} (as expected)

merge(false, (previous) => !previous); // false (expected to see true)

I think the ability to handle primitive values as the first argument will make a more consistent behavior.

fuzetsu commented 4 years ago

Thanks, I'm glad you like it! Actus looks cool 😄

You make a good point about this being more consistent behavior.

I actually wanted mergerino to work this way originally, but couldn't really come up with use cases for it beyond demos and it bulked up the code a little bit more because I had to add non object target handling to the merge function.

I'm not against implementing this kind of behavior, but I'm curious if you have any particular use cases in mind? I've been using mergerino pretty extensively in my own projects and haven't felt the need for this yet.

Or do you think the consistency is worth it just from an API perspective?

EvgenyOrekhov commented 4 years ago

Thanks for checking out actus :)

I already added the ability to use mergerino (or a similar library) with actus by providing a custom function to it with the following signature: getNextState: (previousState, actionResult) => nextState.

So to set up actus with mergerino, you need to add one extra line to your actus config: getNextState: merge.

And then, instead of returning the next state, your actions are supposed to return a mergerino patch.

Here you can see an example action from one of my pet projects.

Without mergerino (has to return the next state, uses Ramda functions for that):

setContent: (content, state) =>
  pipe(
    evolve({ tasks: setContentAtPath(state.editingContentPath, content) }),
    mergeLeft({ editingContentPath: [] })
  )(state),

With mergerino (returns a mergerino patch, which is then getting applied by actus to produce the next state):

setContent: (content, state) => ({
  tasks: setContentAtPath(state.editingContentPath, content),
  editingContentPath: [],
}),

Really clean and declarative, I really like the mergerino version.

Regarding your question, I actually think this "primitive values" use case might be pretty common in actus.

actus has a concept of "slice actions" (akin to Redux'es slice reducers, only built-in).

For example, when you have the following state:

state: {
  someValue: 123
}

you can nest your action using the same property name (someValue), and it will receive not the whole state, but only that someAction slice, 123 in this case:

actions: {
  someValue: {
    someAction: (value, state) => ... // state will be 123, not the whole state object
  }
}

Here you can see slice actions in action (pun intended :)): https://codesandbox.io/s/actus-mergerino-4xs7z?file=/src/index.js

Now, originally I expected that mergerino would just work in this case, but if you uncomment line 11 that enables mergerino, you will see that it doesn't work. Well, the toggle button actually works the first time you click it, but you can see in the browser console that the value becomes {}, not true.

Bottom line, I don't have enough experience to say whether it will be common use case in actus, but I suspect so.

Not sure about other usages of mergerino, but I do also think that consistency is worth it just from an API perspective.

EvgenyOrekhov commented 4 years ago

To clarify the above example, a typical actus app is expected to use all kinds of actions: top-level actions that work with the whole state object, slice actions that work with subobjects, and slice actions that work with "leafs" (primitive values and arrays). And the goal is to make all types of actions behave the same way.