search

ljharb / qs

A querystring parser with nesting support
BSD 3-Clause "New" or "Revised" License
8.5k stars 729 forks source link

rename option `filter` to `filterMap` #448

Closed gomain closed 2 years ago

gomain commented 2 years ago

Because that's what it is. See https://github.com/ljharb/qs/issues/446#issuecomment-1183165378

ljharb commented 2 years ago

There’s nowhere near enough value in the rename to be worth the churn of changing it.

gomain commented 2 years ago

Maybe I was not being persuasive enough. The value lies in being consistent with the functional paradigm that is an exponentially growing trend. The behavior of filter, overloaded to support mapping, is at best surprising. This behaviour is firmly established by the term filterMap: the composition of filter then map. filter, on the other hand, is firmly established to not map. By having this option named filter we obfuscate this functionality leading to #446. #447 is an intermediate remedy but not ideal.

The ultimate api would be that filter strictly takes a predicate, map (or perhaps transform) takes a custom transformer, where transforming to string by passes any other qs transformations (except encoding). Or a single filterMap, the combination of both. Having both map and filterMap would complicate in regards to order of execution.

It would take some grace period to depreciate the existing filter.

I can't argue that the churn is worthwhile. But should this be decided based on principle rather than demand? Please consider.

ljharb commented 2 years ago

I agree that what the option does, in an FP world, would be named "filterMap".

Breaking changes are very costly, so dropping use of the name "filter" is not worth it. Adding "filterMap" and deprecating "filter" would avoid this problem, but it would then create confusion - since both would need to be documented.

The "filter" option was added in #81, over 7 years ago. Since then, out of the 67 million people downloading it a week (these aren't likely unique, so it would be incorrect to extrapolate that to 24 billion :-p), you are the only person that has ever raised the question of the naming of this option.

So, certainly if it was based on demand, it would be an immediate no. Basing it on principle, while I agree that perhaps "filterMap" would have been a better initial name, now that "filter" is established, I think that the principles of minimizing churn and confusion and breakage far outweigh the academic principle of FP correctness. Not everything needs to be a monad or a functor.

gomain commented 2 years ago

Thank you for the thorough explanation. I'm well aware of the numbers, and the distribution of academics.

For the record, Monads and Functors are discovered not designated. We either accept or ignore.