xodio/hm-def

Is a good idea to use lodash sub-packages ?

danielo515 opened this issue · 5 comments

Hello,

One of the things introduced on the great PR #8 was some lodash deps. I understand that the intention was to minimize the dependencies introduced by using just two subpackages of lodash instead of the entire lodash.
However, I see this as an anti pattern for 3 main reasons:

  1. Lodash is a very spread dependency, and if any of your dependencies includes lodash you will end with the entire lodash library plus your independent packages, just the opposite you want.
  2. Independend lodash packages are pretty outdated and they do not have proper documentation
  3. Individual packages have been discontinued by lodash author (see lodash/lodash#3565 (comment))

So it may be a better idea to just use lodash.
What do you think ?

nkrkv commented

Why did Lodash ever appeared among the dependencies? cond and fromPairs were initially provided by Ramda.

Ramda is not safe, Sanctuary is better, let’s get rid of Ramda in favor of Sanctuary. That was a good idea for #8

But Sanctuary misses a few functions from Ramda. They are quite trivial and are re-implemented on the top of signature.js:

//    isEmpty :: Foldable f => f a -> Boolean
const isEmpty = xs => xs.length === 0;
//    propEq :: String -> a -> StrMap a -> Boolean
const propEq = prop => val => obj => obj[prop] === val;
//    indexBy :: (StrMap a -> String) -> Array (StrMap a) -> StrMap (StrMap a)
const indexBy = memoize
  (f => S.reduce
    (xs => x => S.insert (f (x)) (x) (xs))
    ({}));

The missing cond and fromPairs look trivial too and IMO it is better to just re-implement them the same way and drop the lodash.* dependencies altogether.

PRs are welcome.

I originally replaced them in a (somewhat misguided) effort to get rid of Ramda. At the time, I didn't bother to just write them myself, but, looking at them now, I see they aren't complicated at all. So I wrote them just now.

//    fromPairs :: Array (Array2 String a) -> StrMap a
const fromPairs = S.reduce
  (acc => curr => S.insert (curr[0]) (curr[1]) (acc))
  ({});
//    cond :: Array (Array2 (a -> Boolean) (a -> b)) -> a -> b
const cond = conds => x => {
  const c = conds.find (y => y[0] (x));
  if (c !== undefined) {
    return c[1] (x);
  }
  throw new Error (`No predicate was satisfied for ${x}`);
};

Unsure whether cond should return undefined or throw if none of the cases match though. Thoughts?

nkrkv commented

Oh, thanks. For the strictness and definite return value type exception is fine I think.

My thoughts exactly.

This signature is misleading:

//    isEmpty :: Foldable f => f a -> Boolean
const isEmpty = xs => xs.length === 0;

One of the following would be accurate:

//    isEmpty :: { length :: Number } -> Boolean
const isEmpty = xs => xs.length === 0;
//    isEmpty :: Foldable f => f a -> Boolean
const isEmpty = xs => S.size (xs) === 0;
//    isEmpty :: Monoid a => a -> Boolean
const isEmpty = xs => S.equals (xs) (S.empty (xs.constructor));