consider having methods returning cloned objects added to Record rather than Object

[DerekNonGeneric]

I was reading the tentative API described in the README and wonder whether there has been much thought put behind the current return values of these methods.

Returns a new object, which has picked or omitted properties from the object.

Is there any reason it returns a new object rather than mutating the original object? I am not sure what justification there is for returning a new object here, but I can think of some reasons why it would be a bad idea to return a different object than the modified original. For example, if the original object is gigantic, memory constraints may prove to be prohibitive in some scenarios, and simply modifying the original would be more favorable instead. Also, what dictates whether the object returned is an empty object or some other similar variation of an Object instance? We can answer this question by returning the mutated original. [nvm, likely to introduce hard-to-catch bugs] I know Lodash and several other utility libraries (own included) return a copy, but it may be unjustified aside from convention. Anecdotally, I plan on introducing a breaking change to the omit function found in @openinf/util-object to correct this behavior in v3.

Looking at the change-array-by-copy proposal, the justification for not returning the original array in that proposal seems to be to enable dealing with frozen arrays¹. I am not sure whether folks interact with frozen objects on an equal basis; it should probably be noted that these change-by-copy methods are intended to exist on Tuple rather than Array, which makes me think that the change-by-copy variations of these methods (that return a new object) should probably exist on Record². In contrast, the ones on Object should mutate the original object passed. It seems like the ones returning a new object would be more appropriate on Record since it would allow them to operate on their deeply immutable Object-like structure…

Record

a deeply immutable Object-like structure

Tuple

a deeply immutable Array-like structure

P.S. Really cool to see that @js-choi is now championing this one; hopeful we can work together — cheers!

Footnotes

1. https://github.com/tc39/proposal-change-array-by-copy#motivation ↩

2. https://github.com/tc39/proposal-record-tuple#overview ↩

[ljharb]

Mutation is a bad practice and we should avoid it - and new APIs shouldn't encourage it.

[DerekNonGeneric]

Mutation is a bad practice and we should avoid it - and new APIs shouldn't encourage it.

Yeah, you might be right. The mutating Array methods did not go over too well. Brings back memories of when I tried to add a lint rule for them in the website repo (hehe) tc39/tc39.github.io#213 (comment). If they had to exist on Object, perhaps distinguishing that they do not mutate could be solved via naming:

• clonePick()
• cloneOmit()

Refs: https://twitter.com/seldo/status/1196252039828738048

[aleen42]

@DerekNonGeneric In my opinion, any mutation of the original object should be explicit rather than depending on the API implicit behaviour, like:

let obj = {a : 1, b : 2, c : 3};
obj = Object.pick(obj, ['a']); // => try to mutate the object

But not:

const obj = {a : 1, b : 2, c : 3};
Object.pick(obj, ['a']);

// try to assume that the object has been mutated

[DerekNonGeneric]

I have re-framed this issue somewhat since it seems to shed some relevant light on a valid point by drawing parallels to the current logic being used justifying the Change Array by Copy methods being added to Tuple of the Record & Tuple proposal, which would seem likewise to justify changing these couple of methods to Record as they would work well on immutable objects.

---

any mutation of the original object should be explicit rather than depending on the API implicit behaviour

@aleen42, I am of the current opinion that mutation should only be possible by explicitly opting into that behavior. That seems to be what you are getting at above (since let and const do not affect the mutability of objects). [AFAIK, the biggest fail of the mutating Array methods seems to have been that nobody was aware of what was happening — opting into it may help alleviate this (unless mutating methods are always an antipattern since they would no longer be considered “pure“ functions causing the FP community to be upset ¯\(ツ)/¯).]

[aleen42]

I think the proposal should consider whether those methods should return a new Record rather than an Object when we try to pick or omit properties from a Record. I think it depends when the Record/Tuple proposal has been pushed towards state 4. @ljharb @hemanth

[ljharb]

I don't think that will change anything. Just like the presence of Map and Set didn't change that Objects and Arrays remain the preeminent patterns developers prefer to use, Record and Tuple won't likely do so either - at least on a fast enough timeline for this proposal to be affected.

This is Object.pick etc, not Record.pick - that would be a separate proposal.