R.omit allows string (not array)

[wclr]

So this should be prevented with typings, because one may think that one may pass string if there is only one property to omit.

[ikatyang]

Ramda allowed ArrayLike to be passed in R.omit, i.e. the following two cases are considered the same:

R.omit('ad', {a: 1, b: 2, c: 3, d: 4}); //=> {b: 2, c: 3}
R.omit(['a', 'd'], {a: 1, b: 2, c: 3, d: 4}); //=> {b: 2, c: 3}

And AFAIK there's no way to allow ArrayLike but disallow string since string satisfies with ArrayLike.

[KiaraGrouwstra]

I can verify @ikatyang's explanation here; disallowing string here would simultaneously break input using e.g. ImmutableJS List, though I'd considered the string variant a legitimate under-documented feature of Ramda as well. I get where you're coming from here though @whitecolor. Maybe we should be open to PRs for a branch simplifying ArrayLike to Array for those who'd value the clarity over flexibility?

[wclr]

Well in docs it is stays clearly Array. And it assumes a list of props.

Why ArrayLike in typings is used not just Array?

[KiaraGrouwstra]

I at the time opted to err toward a descriptive approach (-> allow what works in Ramda) over a prescriptive one (-> allow what Ramda documents), figuring it'd cover more legitimate use-cases, notably alternate array implementations.

As Ika noted, one of the challenges here is it's actually kinda hard to allow those while disallowing string here (though string also works in Ramda, at which point one might argue it to be legitimate).

I'd be open to having branches for both approaches, and if people would prefer the clarity over flexibility by default, I wouldn't object to having your version as the default. I suppose one might argue those that want the ArrayLike support would be power-users, while for new users it may admittedly make sense to prevent accidental misuse.

[wclr]

String works but it really should be not allowed by typing to avoid silly errors and disambiguation.

It is a common case to omit one prop and people will often make mistake like:

R.omit('secret')

String here is really non-common use case and people always can do it:

R.omit('abcd' as any) 
// or 
R.omit('abcd'.split(''))

One in a million "power user" probably will. =)

[wclr]

So will be fixed?

[KiaraGrouwstra]

I'm not actively working on this repo; Ika I think had been more concerned about getting a DefinitelyTyped compatible release out.

[ikatyang]

In the R.omit case, I think it's fine to remove ArrayLike support since nobody uses ArrayLike to store keys and it can avoid the 'ad' === ['a', 'd'] confusing.

[KiaraGrouwstra]

Might it be elegant to have one branch (set) say type List<T> = ArrayLike<T>, another type List<t> = Array<t>?

[ikatyang]

It's okay to me, but won't it become more confusing to newbies? and I think our goal is to provide one most useful branch instead of adding more branches (if DT is available, we can just put one version there).

[KiaraGrouwstra]

These are good questions.

won't it become more confusing to newbies?

I guess that could be said of either:

• many branches (needs docs to explain)
• the current behavior, consistent with Ramda but deemed confusing by the OP (and presumably others)
• inconsistent behavior (handling omit different from other functions)

I think our goal is to provide one most useful branch instead of adding more branches (if DT is available, we can just put one version there).

Yeah. How would you see it then? It sounds like we're both trying to consider new users here, yet reaching different conclusions on how to do that.

[ikatyang]

I personally prefer the current behavior but the omit case is really confusing with string as the first parameter, or we'll have to wait for the subtraction type support (Microsoft/TypeScript#4183), which is not on the TS roadmap.

---

The reason we have 4 branches is about the performance concern, but it seems it's not a problem now? then I'd like to remove other branches since we'll send only one branch PR to DT eventually.

[KiaraGrouwstra]

The reason we have 4 branches is about the performance concern, but it seems it's not a problem now?

If so then great.

If there's demand for different branches I don't mind. Guess if so we'll hear about it.

So different trade-offs among branches (what did I miss?):

• ArrayLike: enabled #113 (ReadOnlyArray) and #115 (ImmutableJS) but also strings, causing correct (descriptive) yet confusing behavior (this thread).
• placeholder overloads: enabled R.__ but gave many overloads. so far no complaints about performance though.

I guess there will be different users with different preferences, though admittedly maintenance complexity is a consideration as well. If alternate branches could bring options for power users without imposing a burden on maintenance, that'd sound nice as long as some readme notes could tell people what's what. That said, you're the one doing most of the work here now, so I respect your preference.

[ikatyang]

If it's just a simple option (e.g. ArrayLike -> ReadonlyArray, which seems no need to test it), I don't mind to add branches for it.

From GitHub searching, there's no one uses branches other than dist, so I think it should be safe to drop other branches (no updates for them and remove mention on README)? and the future branch will look like:

• dist
• dist-strict-array

And if there's more options in the future, we'll just have one-enabled and all-enabled branches, e.g.

• dist
• dist-strict-array
• dist-some-option-1
• dist-some-option-2
• dist-all-enabled

[KiaraGrouwstra]

Fair enough. :)

[wclr]

i'm not sure, but those branches look like the unnecessary complication

[KiaraGrouwstra]

well, if we drop the features altogether, wouldn't #113 and #115 just get refiled? it'd sound like fixing one issue and breaking two others again. who's to say which use-cases are legitimate, and which aren't?

that said, if we have usage stats for branches, then if it turns out no-one uses them we can still just ditch them. heck, I do imagine usage would be marginal, so perhaps a readme note of "change this List definition to use ArrayLike" could suffice...