Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upBREAKING: Support for Array and plain Object (adds functional API) #1369
Conversation
9ddf658
to
7e7ada5
This comment has been minimized.
This comment has been minimized.
jpollard-cs
commented
Oct 13, 2017
|
This is awesome! Why not have the data/collections come after the other arguments to be more curry friendly? |
This comment has been minimized.
This comment has been minimized.
It's a tradeoff. Use of curry is still pretty uncommon in JavaScript programs, so the argument order intends to better fit the most commonly expected order from JavaScript devs instead. I don't think it would be wise to choose a different order than similar methods in Also, if I understand correctly the datastructure-last argument order for curry friendliness makes the most sense within languages which have syntactic support for currying. For languages that do not, like JS, the order should play a less important role - you'll need to use some explicit function to curry anyhow. For example: const first = collection => get(collection, 0);
first(['a','b','c'])Or if you use a function like const get = ramda.curry((key, collection) => Immutable.get(collection, key));
get(0)(['a', 'b', 'c']) |
This comment has been minimized.
This comment has been minimized.
jpollard-cs
commented
Oct 13, 2017
•
|
True - one could always provide a wrapper function. You make good points. I guess I was under the assumption that if you're going out of your way to use a functional API you would expect the data to come last, but perhaps lodash FP is more popular than I think (for the record I love and used lodash for many years, but prefer ramda over lodash FP for the above reasons). |
| let i = 0; | ||
| while (i !== keyPath.length) { | ||
| // Intermediate null/undefined value along path | ||
| if (!collection) { |
This comment has been minimized.
This comment has been minimized.
danprince
Oct 13, 2017
Seems like this will cause problems with other falsy values. For example:
let names = ["foo", "bar", ""];
names.map(name => getIn(name, "length"));
// will return
[3, 3, undefined]Maybe it'd be better to use collection == null or be completely explicit with collection === null || collection === undefined.
This comment has been minimized.
This comment has been minimized.
leebyron
Oct 13, 2017
Author
Collaborator
Thanks for the review! You're right that this is a strange outcome. Instead, I think getin() should treat non-data structure values as opaque, which will control the set of outcomes. I'll follow up
This adds a functional API for the core read/write methods (get, set, update, merge) (originally proposed in #39). I don't believe that simply providing a functional alternative is enough value to warrant the increase in API surface area, but what this functional API enables is to use the immutable read/write API on typical Array and Object values, allowing for a combination of nesting Immutable Collections and typical Arrays and Objects while preserving the ability to use `getIn` and `setIn`. As an example: ```js import { getIn, setIn } from 'immutable'; const data = { x: { y: 'z' } } const z = getIn(data, ['x', 'y']) const data2 = setIn(data, ['x', 'y'], 'abc') // { x: { y: 'abc' } } ``` I think that providing this flexibility will allow for a more reasoned use of Immutable collections based on best fit, rather than the "all or nothing" approach often taken. Specifically, I'm interested in supporting typical Objects as a viable alternative to Record for those that want to use Immutable collections but prefer not to use the Record API.
…return self for no-ops
…sistent behavior.
This comment has been minimized.
This comment has been minimized.
mattzeunert
commented
Dec 29, 2017
|
Could all the affected functions be added to the changelog? I ran into this with |
Immutable now calls toJS deeply even on immutable values contained in a
native array or object. This was broke displaying child notes, e.g. for
`Immutable.Record({bar: new Immutable.Map({a: 5}) })`.
immutable-js/immutable-js#1369
This comment has been minimized.
This comment has been minimized.
WorldMaker
commented
Jun 4, 2018
|
I wanted this functionality in a couple places, but didn't quite realize how much it would impact elsewhere in the existing application as I was using POJOs as "merge barriers" in a bunch of places. I'm trying to square how to mix both functionality now. Now that POJOs are no longer "merge barriers", I'm wondering if there needs to be some other way to set explicit "barriers" when using something like |
leebyron commentedOct 10, 2017
•
edited
BREAKING: This changes the behavior of a few common methods with respect to Arrays and plain Objects:
.merge()- where previously plain object or array values were considered opaque and replaced completely, they are now merged with the incoming value..getIn()/.setIn()/.updateIn()- where previously key paths that encounter plain object or array values would throw an error, replace a value, or return undefined, they now key into the plain object or array values..toJS()- where previously plain object and array values were considered opaque and returned by reference, toJS() will now return copies of such values, and recurse into them converting nested immutable collections to plain JS values.This adds a functional API for the core read/write methods (get, set, update, merge) (originally proposed in #39). I don't believe that simply providing a functional alternative is enough value to warrant the increase in API surface area, but what this functional API enables is to use the immutable read/write API on typical Array and Object values, allowing for a combination of nesting Immutable Collections and typical Arrays and Objects while preserving the ability to use
getInandsetIn.As an example:
I think that providing this flexibility will allow for a more reasoned use of Immutable collections based on best fit, rather than the "all or nothing" approach often taken. Specifically, I'm interested in supporting typical Objects as a viable alternative to Record for those that want to use Immutable collections but prefer not to use the Record API.
This does add some byte-weight to the library, but only marginally due to replacing existing implementations. Adds +209 gz-bytes, or +1167 raw-bytes.
Remaining tasks:
Closes #1268
Closes #1363