Array additions

[glennsl]

This adds:

• make, seems necessary to be able to create initialized arrays without imperatively pushing onto an empty array.
• init, is make with an initializer function instead of a constant.
• findMap, is surprisingly useful, to find an element in a nested array, for example ([[1, 2], [3]]->findMap(find(_, el => ...))), or to find the first element in an array<option<_>> that is not None.
• keepSome, unwraps the options in an array<option<_>> while discarding the Nones.

It also replaces the reimplementation of flatMap with a binding to Array.prototype.flatMap.

This is, to begin with, just to gather some feedback on the relevance, naming and implementations, before I add documentation and tests.

TODO:

• Documentation
• Tests
• Changelog

[cknitt]

Thanks! Here is some feedback from my part.

What do you think @zth?

[zth]

Thanks @glennsl, these additions look quite useful. I added my comments to the discussion as well.

[glennsl]

I suppose this could be merged now, since neither documentation for the rest of the module or a test framework is in place yet. It could also be what initiates both. Up to you!

[zth]

If you don't mind, I think adding both in this PR would be good, so it doesn't get left behind. Would that be OK? Let me know how I can help out if needed.

[zth]

Also, a change log entry would be good as well.

[glennsl]

Sure. Do you have any thoughts on testing framework? Should I pull in something like rescript-jest or make something very simple and bespoke?

[zth]

We have a simple one already in Test.res that we can start by using. Check out PromiseTest.res for how to use it.

Thank you for all your contributions so far by the way, very valuable! 👍

[glennsl]

Added documentation, tests and updated the changelog. Also fixed reduce and reduceReverse, which were reimplemented instead of binding to its native counterparts. That did require changing the type signatures and renaming reduceReverse to reduceRight though.

[zth]

Great! Taking a look again soon.

[zth]

Actually, reduce and reduceReverse were intentionally taken from Belt instead of using their native counterparts because the Belt version allows for full type inference whereas the native one does not. So that was a deliberate decision.

Just had a small comment, other than that and the reduce stuff this is good to go from my end. Thanks!

cc @cknitt if you want to have a last look as well.

[cknitt]

Actually, reduce and reduceReverse were intentionally taken from Belt instead of using their native counterparts because the Belt version allows for full type inference whereas the native one does not. So that was a deliberate decision.

If it's just for the improved argument order for better type inference, then we could simply do

@send external reduce: (array<'b>, ('a, 'b) => 'a, 'a) => 'a = "reduce"

let reduce = (arr, x, f) => reduce(arr, f, x)

(same for reduceRight).

[glennsl]

Indeed. Seems odd to have them reimplemented and with different names when only a reordering of the arguments is needed.

Could probably also just label the argument to allow it to be moved before the callback function?

@send external reduce: (array<'b>, ('a, 'b) => 'a, ~init: 'a) => 'a = "reduce"

[zth]

IIRC the Belt version actually has better performance than the native one. Anyway, just reordering the arguments like that is fine. Let's do that instead and axe the reimplementation.

Side note - it's entirely possible that we'll have a way to do this directly in the binding in the future too, which would make it zero cost. Not there yet though.

[glennsl]

So, reorder, not label?

[zth]

Correct.

[glennsl]

Arguments reordered! Also reordered the type variable names for consistency.

[zth]

Fantastic, great work @glennsl ! Good to go from my end. @cknitt ?

[cknitt]

Yes, great work! I think it would be good to use pipe -> in the examples, but that can also be done separately later as it applies to other modules, too. Let's merge now! 🙂