brianneisler / mudash

Lodash wrapper providing Immutable.JS support
http://mudash.org
MIT License
107 stars 4 forks source link

Always immutable (remove mutation special casing) #4

Open chrisui opened 8 years ago

chrisui commented 8 years ago

I saw the note in the readme and just wanted to have an issue where desire for this can be tracked and an area for discussion. This would be my expectation from such a library - I would expect whatever I pass into these utilities to be immutably handled (whether an immutable.js data structure or other).

From the readme:

In the future we may opt for an entirely immutable approach for both forms of data, similar to the approach for lodash-fp

+1 from me

brianneisler commented 8 years ago

Thanks @chrisui for opening this issue for discussion. Definitely an important topic.

On one hand I see some significant benefits of taking an entirely immutable approach.

let map = Immutable.Map({}) setA(map) // Map {} ???

function setA(val) { _.set(val, 'a', 1) return val }


This is a rather contrived example but is valid in scenarios where you may want to handle both types of data. Allowing for mutability opens a door with a lot of error prone possibilities where it works for one data type and not the other.

On the other hand, my biggest concern with the immutable approach is the divergence from the lodash spec. This prevents mudash from being a drop in replacement for lodash. 

I suppose one option would be to default to immutability and then allow for this functionality to toggled on/off through some mechanism similar to lodash fp's convert method https://github.com/lodash/lodash/wiki/FP-Guide#convert
miangraham commented 8 years ago

Awesome library. The absence of something like this is the main reason I haven't been using immutablejs.

Chiming in to say that, in a dynamically typed environment, pivoting between side effects and no side effects at runtime based on the type of object received would make this super hard for me to use with confidence. Unless I wrap all calls with type checks, the same unpredictability expands up into my own code.

Rather than a drop-in replacement, I'd have a much easier time with slightly different method names--or separate modules or whatnot--whose behavior I could predict reliably. Glad to spend the characters calling setImmutable (or whatever) if that's what it takes, especially since I can rename as desired on import. A convert-like approach could work well too.

My first impulse was "that thing should just throw if I ever pass it a non-Immutable," but that may be a bit extreme!

Again, great project and looking forward to using it.

brianneisler commented 8 years ago

Thanks @miangraham!

Rather than a drop-in replacement, I'd have a much easier time with slightly different method names--or separate modules or whatnot--whose behavior I could predict reliably. Glad to spend the characters calling setImmutable (or whatever) if that's what it takes, especially since I can rename as desired on import.

I agree with this sentiment. Seems best to create a predictable immutable api in mudash and import lodash when mutable methods are needed.

brianneisler commented 8 years ago

Potentially targeting this change for version 0.3.0. Assembling a list of the lodash methods here that mutate data and would need to be modified.

TODO: Methods remaining for conversion to "completely immutable"

brianneisler commented 7 years ago

I've taken a first stab at converting a few Lodash methods with a "mutable" signature to an "immutable" one.

The following methods have been converted to be immutable.

Because the pull* methods already have an immutable counterpart in Lodash in the form of the without and difference* methods, I thought it best to introduce some new functionality for the pull* methods. Rather than having them remove ALL matched values from the data set, which replicates the without and difference* functionality, the pull* methods will now remove the FIRST match from the left. Therefore if you have multiple of the same value in a data set you will need to include multiple instances of the value in values param.

Example:

Lodash behavior

const array = ['a', 'b', 'c', 'a', 'b', 'c']
_.pull(array, 'a', 'c')
console.log(array) // => ['b', 'b']

mudash behavior

const array = ['a', 'b', 'c', 'a', 'b', 'c']
const result = _.pull(array, 'a', 'c')
console.log(array)      // => ['a', 'b', 'c', 'a', 'b', 'c']
console.log(result)     // => ['b', 'a', 'b', 'c']

const list = Immutable.List(['a', 'b', 'c', 'a', 'b', 'c'])
const result = _.pull(list, 'a', 'c')
console.log(list)         // => List ['a', 'b', 'c', 'a', 'b', 'c']
console.log(result)    // => List ['b', 'a', 'b', 'c']
brianneisler commented 7 years ago

Just converted the following methods to be immutable. These methods have been released in v0.5.0