Suggestions

[selfrefactor]

This issue is opened to encourage suggestions and feedback.

Vote for bringing even more Ramda methods in Rambda

[selfrefactor]

Deprecate R.contains as Ramda has already did it?

[selfrefactor]

Introduce R.project https://ramdajs.com/docs/#project

[selfrefactor]

R.pickBy https://ramdajs.com/docs/#pickBy

[kuchta]

Could forEach be enhanced to work on Objects as well as Arrays as map and filter do to match Ramda's forEachObjIndexed which Rambda has not counterpart?

[selfrefactor]

Could forEach be enhanced to work on Objects as well as Arrays as map and filter do to match Ramda's forEachObjIndexed which Rambda has not counterpart?

This is nice suggestions. I will post spec once the new behavior is written, so you can confirm it.

[selfrefactor]

@kuchta I will have to ask for collaboration as if you look at the source of forEach, you will see that it already passes to R.map. I only changed R.map implementation and now iterator function accept third argument namely the input object. This change is published with version 2.6.0

[StreetStrider]

Expose src/ as es/ for Rollup, so I could import curry from 'rambda/es/curry'.

[selfrefactor]

@StreetStrider src was removed per previous request, but I don't mind exposing it back as Ramda also exposes its src folder. Can you share reasoning why it should be exported as es folder?

[StreetStrider]

@selfrefactor for me, actual name here is a matter of clarity. Names, like es/, cjs/ or .umd.js are much clear in pointing at format of specific modules, while src/ and lib/ indicates that here are original files (which may be unconsumable by default).

es is better than src, but src is OK too.

[selfrefactor]

I thank you @StreetStrider for the clarification. The latest version 2.7.1 exposes src per your request.

[StreetStrider]

@selfrefactor I've tried npm i in this package today and got difficulties. I can't install all dependencies (erros like pathspec '2.0.0' did not match any file(s) known to git.). Can you confirm that you are able to install dependencies from scratch (rm -rf node_modules then npm i)?

[selfrefactor]

I can confirm the same bug. It seems that you will have to use yarn for this purpose. Another strange case where yarn works where npm does not. I have some other similar cases so nothing new.

[StreetStrider]

I've tried install deps one-by-one, error occurs here:

↳ p i 'selfrefactor/minify#0.1.0'     
 ERROR  Could not resolve 2.0.0 to a commit of git://github.com/sindresorhus/figures.git.

So it is most likely a bug in transient dep of selfrefactor/minify.
Maybe you should try update sindresorhus/figures version.

[selfrefactor]

@StreetStrider I simply removed figures dependency from minify library and now it works with NPM as well.

[StreetStrider]

@selfrefactor good to know. Environment must be reproducible for others.

[romgrk]

Implement lenses functions.

[selfrefactor]

@romgrk this has been brought up before and I will welcome any PR that addresses lenses integration, but I am not using them and it will be a bit hard for me to write the logic.

[romgrk]

Sure, I understand. Brian Lonsdorf has a pretty straight-forward implementation of lenses here, I wonder if we could just re-use that one.
I use rambda for pretty much all my needs, but I do need to import ramda in most projects just to get those lenses.

[selfrefactor]

Thanks for the link @romgrk , but I guess you will have to use ramda for anything regarding lenses. After all the niche of Rambda is to provide partial API for the base methods in ramda. What I can do is to try to apply the suggested link initially in Rambdax and then bring it here. I will write once this initial application is ready.

[romgrk]

Other suggestion, I notice that the signature for some methods is sometimes not exactly the same as ramda. Is it possible to make it 100% compatible? (e.g. ramda.adjust(index, fn, list) vs rambda.adjust(fn, index, list))
Note that the JSDoc of that function (and possibly others) is also wrong (bad signature).
Note that even though the JSDoc says negative indexes are ok for adjust, they're not handled in the function.
Can I submit a PR for these issues? (not the lenses)

[selfrefactor]

Sure, JSDoc annotations is a recent PR which was pretty big and as you noticed there are obvious inconsistencies. Your PR fixing these issues is very welcome.

[romgrk]

Ok, is it fine if I modify rambda's API to match ramda one, rather than modify the jsdocs?

[selfrefactor]

You will have to check the entry in the intro of README.md for the known differences between Rambda and Ramda as this differences are made on purpose.

[romgrk]

They're made on purpose? It would be more convenient for users to have the same API.
But can you just confirm that you're ok if I submit a PR that modifies rambda's API?

[selfrefactor]

It is almost the same. I try to extend rather than change existing behavior. I am fine with modification as long as they are not related to the known differences.

[selfrefactor]

@romgrk Your changes are live with the new version. Thanks for the input and I will keep you updated when there is progress with lenses.

[kuchta]

It would be great to have typings directly in the source code (making the source code effectively TypeScript) to prevent it to get out of sync with the code.

[selfrefactor]

@kuchta I am unsure what you mean. Do you mean something like Flow? Even so, I am unsure how well it works with curried function as this is an issue with Typescript

[kuchta]

@selfrefactor I meant to put typings directly into sourcecode and rename it from js to ts :)

[StreetStrider]

@selfrefactor I looked through your code, and everywhere you use «manual» currying, without explicitly depending on curry. The same trick can be applied to typings both Flow and TS, via function overload, you just need to declare function signature multiple types. Something (pseudocode) like this:

foo(A, B) => R
foo(A) => foo(B) => R

TS overloads.
Flow docs is bad, but it can be done too, via multiple declare function.

[selfrefactor]

@StreetStrider I tried this approach before, but I wasn't happy with the output for Typescript typings(i.e. index.d.ts). But I guess it is possible to keep the current typings and ignore those produced by tsc build. I will give it a try and thank you for your input on the subject.

[selfrefactor]

@kuchta My recent work experience changed the way read any code and now I try to be as simple as possible. This means that I am not very eager to understand and implement lenses, but as I had said before, this doesn't mean that I will reject any PR that addresses this feature request.

[damiangreen]

I'm evaluating rambda as an alternative over ramda for the cases where it doesnt type wel lfor the compose and pipe operators , e.g. (https://stackoverflow.com/questions/54363310/how-to-remove-unnecessary-casting-with-ramda-and-typescript). It still appears to suffer the same drawbacks. Is there any intention to improve upon this?

[selfrefactor]

Still wating for this Typescript issue to be addressed. Until then, there is not much that it could be done in this regard.

Personaly me, I stopped using compose and pipe with my Typescript code as there is no easy way around it.

One crazy solution is to have two methods for curried and uncurried version, i.e. R.foo and R._foo and then we'll be able to write this library with Typescript and let it create the final typings. But this is mad science and I brought it just to illustrate even further that currently this is not a simple matter.

[squidfunk]

Ramda methods I miss in Rambda:

• intersection, intersectionWith
• difference, differenceWith
• intersperse
• lensPath
• set

[selfrefactor]

As stated before, lenses are hard for me to implement, so unless somebody opens a PR, they will be lacking in Rambda.

As for the other 5 suggestions - I will implent them with the version after the next one as now I am in the middle of writing typings tests.

Thank you for your suggestions @squidfunk

[squidfunk]

I went ahead and implemented the set functions in four separate PRs. They can be generalized to the *With versions later on. I also tried to stick to your coding style. Would love to see them in the next release :-)

[selfrefactor]

Great job @squidfunk
I approved all of PRs and I will try to make a release this weekend, even though I am not completely ready with typings refactor caused by typings tests. Thanks again.

[squidfunk]

That's great news! I also realized that I can simplify a situation where I used lensPath and set to assocPath. Will try to submit a PR for this, too.

[squidfunk]

There was an error merging the typings - missing /*:

rambda/index.d.ts

Lines 351 to 355 in 33ed7fc

	* Combines two lists into a set (i.e. no duplicates) composed of those elements common to both lists.
	*/
	intersection<T>(list1: ReadonlyArray<T>, list2: ReadonlyArray<T>): T[];
	intersection<T>(list1: ReadonlyArray<T>): (list2: ReadonlyArray<T>) => T[];

[selfrefactor]

I think the last warning is fixed as tests are passing(including the typing tests)

[squidfunk]

@selfrefactor I fixed it in my PR ;-)

[gotenxds]

Any way we can add set as an alias to assocPath ?
Maybe even get and getOr as aliases for path and pathOr some coming from lodash would find this easier to read.

[gotenxds]

Can we make indexBy accept a Path | (a: T) => string so we can use it like this ?

indexBy('id', [{ id: 1 }, { id: 2 }, { id: 3 }])

[selfrefactor]

For the aliases - my issue is that they shouln't be a first class methods and this brings many other problems. For example, how I update all the aliases when a Typescript definition change? Should I leave them out of Ramda definition file?

I will keep your suggestion alive, but for the moment I have to decline it. Still, it is very possible that this change in the future. I will inform you if that happens.

As for indexBy - can you post your expectations for your example?

[gotenxds]

indexBy could take either a function or a path to the property
so you could either do this:

indexBy(obj => obj.id, [{ id: 1 }, { id: 2 }, { id: 3 }])
// same as
indexBy('id', [{ id: 1 }, { id: 2 }, { id: 3 }])

this could be dont using path so it would support nesting

indexBy(obj => obj.a.b.c.id, [{ a: { b: { c: { id: 1  } } } }, { a: { b: { c: { id: 2  } } } }, { a: { b: { c: { id: 3  } } } }])
// same as
indexBy('a.b.c.id', [{ a: { b: { c: { id: 1  } } } }, { a: { b: { c: { id: 2  } } } }, { a: { b: { c: { id: 3  } } } }])

[selfrefactor]

Ok, that helped.
You can check this file 'src/indexBy.spec.js' where I added a specification for your suggestion. I will try to move it forward with Typescript definitions and the new functionallity should land with version 4.4.0

[gotenxds]

Great ^^ if you need help with that I can make a small PR for that stuff

[selfrefactor]

I am almost there so I guess there is no need for PR. I just have one more task before releasing 4.4.0 but I should be ready soon.

[devdoomari3]

maybe this can be "ramda/fp" like "lodash/fp" ?
(and become part of ramda and make ramda-team maintain this? 😉 )

searching for "rambda js" after reading blog-post gives me results for "ramda js" 😞

[gotenxds]

maybe this can be "ramda/fp" like "lodash/fp" ?
(and become part of ramda and make ramda-team maintain this? 😉 )

searching for "rambda js" after reading blog-post gives me results for "ramda js" 😞

Ramda is already fb, the main difference between rambda and ramda is tree shaking support out of the box and better performance.
Only thing missing now is full TS support as currently it causes compilation errors, @selfrefactor move to TS cough cough ;)

[selfrefactor]

@gotenxds as for your previous comment - please post solution. In other words - you can edit directly Rambda's index.d.ts file in your node_modules and confirm what change fixes the error. I think that the change is the same as my last commit a few minutes ago, but I just want to make sure of it. Also I will do a release once we agree on a fix.

[jcdietrich]

I would like to see cond added.

[selfrefactor]

@jcdietrich Will do

R.cond is useful method - #373

[please-rewrite]

Any chance chain (fantasy-land) can be added to this library. 😬

[tjklemz]

Any chance chain (fantasy-land) can be added to this library. 😬

Ditto. https://ramdajs.com/docs/#chain

[selfrefactor]

@please-rewrite @tjklemz Sure, no problem.

I created issue for that - #408 but it will take at least two weeks until it is implemented, as right now I make big changes in terms of supporting scripts(build documentation, build Rambdax, generate typings, sync with Ramda test, increase dtslint tests).

The feature should be implemented with 5.1.0 and I can ping you once that happen.