remeda / remeda

A utility library for JavaScript and TypeScript.
http://remedajs.com
MIT License
4.42k stars 136 forks source link

Add optional `obj` parameter to `Evolver`? #615

Open Brodzko opened 5 months ago

Brodzko commented 5 months ago

Would it make sense for the Evolver to accept the entire object as a second parameter? This would be useful if you need to use other properties to evolve one. A simple example would be to evolve

const person = {
    firstName: 'John',
    lastName: 'Smith',
    fullName: '',
}

using evolver

const evolver = {
    fullName: (_val, obj) => obj.firstName + ' ' + obj.lastName,
}

I'm aware this example is a bit too simple, but I have many use-cases where I want to manipulate properties conditionally based on values of other properties (before evolution, that is).

Or is there a way to achieve this simply already? The only way I could think of is to have an "evolver factory" which closes over the original value of the object like this:

const createEvolver = <T>(obj: T): Evolver<T> => ({
    fullName: (val) => !val.length ? obj.firstName + ' ' + obj.lastName : val, // updates `fullName` if not set yet
});

and then using it like this

const evolvedPerson = R.pipe(
    person,
    R.evolve(createEvolver(person))
)

Edit: typo

cjquines commented 5 months ago

i like this, but this is a breaking change in cases like:

R.pipe(
  { a: { b: 0 } },
  R.evolve({
    a: R.merge,
  }),
  value => value.a({ c: 1 }),
);

(in particular, this would work now, but would break if we did add the optional obj parameter, as then value.a would be an object and not a function)

Brodzko commented 5 months ago

Yes, I see. It could get considered for v2 maybe? I don't think it would hold its own as a standalone function, so either that or the factory workaround I guess.

Slightly unrelated, but would you consider exporting (at least some) types from remeda? Would be nice if I could enforce the <T>(obj: T): Evolver<T> signature on my factory 🙃

Brodzko commented 5 months ago

Hmm thinking about it some more, maybe it should actually be considered a special case of "evolve A based on values of B", for which the factory approach seems like a recommended solution, so maybe this isn't that necessary.

eranhirsch commented 5 months ago

It's hard to answer without more context.

I was supportive of adding evolve because it is part of Lodash, and I felt there are cases where it might produce more readable code, but it's on the far end of what I define as a utility function (vs. a "solution" function). As an example, we are deprecating a few functions in v2 because they could be easily composed of existing functions, e.g., reject(p) is filter(isNot(p)). In the future, I want to make sure the functions we add are "atomic" so they can't be "devisable" using other utilities.

This is to say that if you need the object in your evolvers, I wonder if you'd be better off simply doing the mutations "bare-bones." It'll be more readable to anyone on your codebase who doesn't know how evolve works. The only line here which is shorter using evolve than the bare-bones impl is the simple one that simply mutates a prop based on it's own value.

const x = pipe(
  ...
  (myObj) => ({
    ...myObj,
    justAConst: 3,
    mutatedProp: myObj.mutatedProp + 3,
    fromAnotherProp: myObj.isAnotherProp ? 3 : 4,
    aCombination: myObj.a + myObj.b - myObj.aCombination + 3,
  }),
  ...
);

// Instead of
const x = pipe(
  ...
  evolve({
    justAConst: constant(3),
    mutatedProp: add(3),
    fromAnotherProp: (_, { isAnotherProp }) => isAnotherProp ? 3 : 4,
    aCombination: (aCombination, { a, b }) => a + b - aCombination + 3,
  }),
  ...
);