selfrefactor/rambda

Mutable API

ivan-kleshnin opened this issue ยท 15 comments

Rambda's reverse modifies the array, instead of returning reversed copy of it.

Just wanted to ask why? When you create a new reversed array it's probably just a shallow copy. There's no need to "fight" for RAM too much as JS already has memory sharing built-in. It's just not protected sharing and we protect it by convention with Ramda approach.

let xs = [{...}, {...}, {...}]
//
let ys = [xs[2], xs[1], xs[0]]

ys does not double the memory usage. It's only additional pointers (4 bytes each)!

I.m.o the convention of "every function here is immutable" has to be kept at any cost.

I agree with your reasoning and I will change that mutable behavior.

Initially I added R.reverse because I created helper method, that directly uses the native method(which mutates the array).

Methods toString, toLower, toUpper and trim uses the same helper.

I will post again, once this issue is solved.

The bug is fixed in the new version 0.9.6

Great!

Btw. Ramda adds ES6 modules in 0.25 version so Webpack / Rollup treeshaking can be applied. Then only functions that are really used can be bundled.

So the purpose of this repo as tinier Ramda can be argued as "worthless" by some people.
As for me โ€“ I think the purposes of having "easier/faster Ramda, with no fuss" are pretty valid. Maybe you have (or will have) some additional motivations to modify Ramda. It's certanly not the only library doing so and I can totally imagine myself using one.

P.S

I would rather drop assoc and assocPath than lenses. It's a shame Ramda lenses have inconvenient API.

I would also added R.id alias for R.identity โ€“ a long requested change in Ramda.

FYI โ€“ just sharing my thoughts at an opportunity.

Thanks for the feedback.

I didn't know that Ramda is now tree-shakeable and you are right that Rambda's argumentation should be changed to reflect this news. If you particular idea for the description of Rambda, please feel free to suggest it as I consider "easier/faster Ramda, with no fuss" more like a general guidance.

R.id alias for R.identity
This seems valid suggestion - will do it.

Can you just explain a bit further what you mean by:

I would rather drop assoc and assocPath than lenses. It's a shame Ramda lenses have inconvenient API.

BTW Which library you are using for tree-shaking?

BTW Which library you are using for tree-shaking?

I didn't try it yet, but I plan to. So I have no idea if it will be easy or not-so-easy addition.
I'll try to remember to leave a note here after that.


Can you just explain a bit further what you mean by:

I would rather drop assoc and assocPath than lenses. It's a shame Ramda lenses have inconvenient API.

I use the following helpers:

let lensify = (lens) => {
  if (lens instanceof Array) {
    return reduce(
      (z, s) => compose(z, typeof s == "number" ? lensIndex(s) : lensProp(s)),
      id, // R.identity
      lens
    )
  } else if (lens instanceof Function) {
    return lens
  } else {
    throw Error(`invalid lens ${lens}`)
  }
}

// Changing global namespace for brevity (bad for libs, ok for apps)
window.R.viewL = curry((lens, obj) => view(lensify(lens), obj))
window.R.setL = curry((lens, val, obj) => set(lensify(lens), val, obj))
window.R.overL = curry((lens, fn, obj) => over(lensify(lens), fn, obj))

Then:

R.assoc("foo", foo, {})
=>
R.setL(["foo"], foo, {}) 

// Btw, setL API can be overloaded to support strings or string masks
// as you did HERE with assocPath. I'm ambiguos about that change. Masks
// have their own problems.

R.assocPath(["foo", "bar"], "fooBar", {foo: {}})
=>
R.setL(["foo", "bar"], "fooBar", {foo: {}})

But lenses are much more powerful because you can

a) modify arrays

R.setL(["foo", "bar", 0], "hi", {foo: {bar: ["_"]}}) // {foo: {bar: ["hi"]}}

Problem: Ramda lenses don't support adding keys to arrays on-the-fly.

b) apply functions with over

R.overL(["foo", "bar", 0], (xs) => xs.concat("hi"),{foo: {bar: []}}) // {foo: {bar: ["hi"]}}

c) get items

R.getL(["foo", "bar", 0], {foo: {bar: ["baz"]}}) // "baz"

d) make your own lenses working with anything: from bit registers to ImmutableJS objects

Too big of a topic to cover here.

The better option for such library as Rambda is to probably just replace R.set, R.get, R.over with the extended versions from above instead of using R.setL suffixed names.

So it's lesser API surface for the extended functionality.

P.S.

There is much more powerful lens library called partial.lenses but it may be too big to recommend "by default".

If you particular idea for the description of Rambda, please feel free to suggest it as I consider "easier/faster Ramda, with no fuss" more like a general guidance.

Good question. To begin with, I would not copy the equivalent API โ€“ it's a lot of work to support.
(especially for Rambda and RambdaX)

I would just made a list of links to Ramda's original API + the difference for functions you repeat or reimplement here. If you're going to keep it "almost API compatible" of course.

Wow- that was a long explanation and I appreciate it.

I am not big on lenses, so I needed some time to comprehend your explanation. Still, I don't feel fully comfortable adding those methods by myself, as I have never used lenses before. I would accept PR with your suggestions, if you feel like it doing it.

As for tree-shaking - I am asking as my attempts with Rollup and Webpack are not as smooth as expected. Will keep the issue open to wait for your experience on that.

As for you latest comment - I agree that it is a lot of work supporting different method explanation, but I see it also as necessary difference between Ramda and Rambda. Also, linking to Ramda original API was actually the very first documentation solution, but the time investment in creating new examples and new method descriptions now is too large to throw it out.

As for you latest comment - I agree that it is a lot of work supporting different method explanation, but I see it also as necessary difference between Ramda and Rambda. Also, linking to Ramda original API was actually the very first documentation solution, but the time investment in creating new examples and new method descriptions now is too large to throw it out.

Easier examples are also fine. Some of Ramda's originals are hard to follow.

I am not big on lenses, so I needed some time to comprehend your explanation. Still, I don't feel fully comfortable adding those methods by myself, as I have never used lenses before. I would accept PR with your suggestions, if you feel like it doing it.

Ok, I'll consider that ๐Ÿ‘

I've succeeded with working treeshaking with Ramda. I'm going to post repo examples soon.
Two important notes:

  1. Bundle size is still big (bigger than Rambda). A lot of currying machinery is pulled for every function in Ramda.

  2. Treeshaking works only for production builds (probably because it's very expensive in CPU). So the only way to keep apps with Ramda fast is to vendor-split it. Bundle size still matters a lot.

As for the API surface question โ€“ I keep collecting the stats on functions I actually use.

I don't use many of fn combinators.

var average = R.converge(R.divide, [R.sum, R.length])
average([1, 2, 3, 4, 5, 6, 7]) //=> 4

vs

let average = (xs) => reduce(R.add, 0, xs) / xs.length
average([1, 2, 3, 4, 5, 6, 7]) //=> 4

I'm yet to find cases where converge-level abstractions are easier to read and write than expanded lambdas. The tip criteria for me is a number of separate function arguments. Two and especially three+ are worth expanding (replacing with lambdas).

// truncate :: String -> String
var truncate = R.when(
  R.propSatisfies(R.gt(R.__, 10), 'length'),
  R.pipe(R.take(10), R.append('โ€ฆ'), R.join(''))
);
truncate('12345');         //=> '12345'
truncate('0123456789ABC'); //=> '0123456789โ€ฆ'

=>

// truncate :: String -> String
var truncate = (s) => s.length > 10 
  ? R.take(10, s) + "โ€ฆ"
  : s
truncate('12345');         //=> '12345'
truncate('0123456789ABC'); //=> '0123456789โ€ฆ'

The first one may be better performance-wise because helper functions are created statically. But I'd still go with the second one by default.

So my proposition for Rambda can be formulated as such:

a) collect usage stats to make rational, fact-based decisions about the API
b) get rid of high-level combinators
c) get rid of low-level array / object modifiers that can be replace with lensing
d) keep API immutable
e) keep API compatible "in large" (which is expected with that name)

With that points, I think, it will represent a different set of tradeoffs and will be preferable for some audiences for sure.

A quick-n-dirty sketch of changes to Ramda (not Rambda) I would apply:

+ map2 = addIndex(map)
+ filter2 = addIndex(filter)
+ reduce2 = addIndex(reduce)
- converge // overthinked
- update // use lenses
- adjust // use lenses
- assoc // use lenses
- assocPath // use lenses
- applySpec // overthinked
- sortBy // sort + ascend/descend is MUCH BETTER
- pathOr // use lenses
- prop // use lenses
- path // use lenses  
...

I've succeeded with working treeshaking with Ramda. I'm going to post repo examples soon.

Really nice. I also played further with tree-shaking and I found out that I need to remove some of Rambda internal helpers, as Rollup otherwise struggles to perform tree-shake. With Ramda and I could create working tree-shaking settings as their helpers also create obstacle for Rollup, so I am very curios how you made that. I will wait for your repo to be ready.

A lot of currying machinery is pulled for every function in Ramda.

One of the reason for me to build my own Ramda - too much unnecessary magic.


As for your suggestions for Rambda - I agree with you that those are required in order to make the library more serious.

With Ramda and I could create working tree-shaking settings as their helpers also create obstacle for Rollup, so I am very curios how you made that. I will wait for your repo to be ready.

I used Webpack. I never tried Rollup โ€“ learning both is too expensive in time, unfortunately.

I am posting a link to a repo, which prove that Rambda is more tree-shakeable than Ramda

https://github.com/selfrefactor/tree-shaking-example

Thank you. Check also this scabbiaza/ramda-webpack-tree-shaking-examples#2 output bundle - 877 B. Looks like there is some magic Webpack plugin that enables a normal tree-shaking :)

@ivan-kleshnin It was nice having this issue-chat but I will close it, as it seems we run out of steam :)