-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mutable API #44
Comments
I agree with your reasoning and I will change that mutable behavior. Initially I added Methods I will post again, once this issue is solved. |
The bug is fixed in the new version |
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. P.S I would rather drop I would also added 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.
Can you just explain a bit further what you mean by:
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 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 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 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". |
Good question. To begin with, I would not copy the equivalent API – it's a lot of work to support. 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 |
Easier examples are also fine. Some of Ramda's originals are hard to follow.
Ok, I'll consider that 👍 |
I've succeeded with working treeshaking with Ramda. I'm going to post repo examples soon.
As for the API surface question – I keep collecting the stats on functions I actually use. I don't use many of 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 // 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 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
... |
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
One of the reason for me to build my own As for your suggestions for Rambda - I agree with you that those are required in order to make the library more serious. |
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 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 :) |
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.
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.
The text was updated successfully, but these errors were encountered: