From 41547079d279dfa0ce61cbd1c30678dee5515376 Mon Sep 17 00:00:00 2001 From: Cheng Lou Date: Mon, 22 May 2023 00:04:48 -0700 Subject: [PATCH 1/2] Use idiomatic new JS array immutable helpers Fixes #138 Added `toSorted`, `toReversed`, `toSpliced` and `with`, per spec. Which means moving `sortInPlace` to `sort`. Same for `reverseInPlace` -> `reverse` and `spliceInPlace` -> `splice`. This is a small breaking change, thankfully caught by the type system, and by Core typing these mutable helpers' return as `unit`. Did the same for `TypedArray`. Though its `reverse` and `sort` _did_ return the array itself, so this breaking change should be carefuly advertized. The `toBla-ed` naming convention seems to be the way to go in JS in the future. We should consider that for us as well. Otherwise we'd end up diverging more and more when JS releases more immutable helpers. --- README.md | 2 +- src/Core__Array.mjs | 17 ---------- src/Core__Array.res | 28 ++++++----------- src/Core__Array.resi | 45 ++++++++++++++++----------- src/Core__List.res | 2 +- src/typed-arrays/Core__TypedArray.res | 9 ++++-- 6 files changed, 45 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index f24e6b97..f0ecc67f 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,7 @@ This standard library is based on `rescript-js`, but with the tweaks and modific - `findIndexOpt`/`lastIndexOf`/`indexOfOpt` are added, returning `None` instead of `-1` if the item searched for does not exist. These are in addition to `findIndex`/`lastIndexOf`, which still returns `-1` when the item you're looking for does not exist. - `getUnsafe` added (copied from `Belt`). - `setUnsafe` added (copied from `Belt`). -- `reverse` added (copied from `Belt`), in addition to existing `reverseInPlace`. `reverseInPlace` is zero cost but does not produce a new array. `reverse` does produce a new array. +- `sort`, `toSorted`, `reverse`, `toReversed`, `splice`, `toSpliced` are the same as their JS counterpart (mutable and immutable, respectively). - `keepMap` is added from `Belt`, but **renamed to `filterMap`**. Rationale: `filterMap` is closer to the JS convention of naming. It's also available in other languages like Rust. `keep` et al can confuse beginners, who're bound to be looking for `filter` style names since that's what JS has. - `shuffle` and `shuffleInPlace` are added (copied from `Belt`). - `flatMap` added (copied from `Belt`, but using native `map` and `concat` functions). diff --git a/src/Core__Array.mjs b/src/Core__Array.mjs index 1560df8f..3a0598ef 100644 --- a/src/Core__Array.mjs +++ b/src/Core__Array.mjs @@ -40,12 +40,6 @@ function lastIndexOfOpt(arr, item) { } -function sort(arr, cmp) { - var result = arr.slice(); - result.sort(cmp); - return result; -} - function reduce(arr, init, f) { return arr.reduce(f, init); } @@ -76,15 +70,6 @@ function swapUnsafe(xs, i, j) { xs[j] = tmp; } -function reverse(xs) { - var len = xs.length; - var result = new Array(len); - for(var i = 0; i < len; ++i){ - result[i] = xs[(len - 1 | 0) - i | 0]; - } - return result; -} - function shuffleInPlace(xs) { var len = xs.length; for(var i = 0; i < len; ++i){ @@ -141,7 +126,6 @@ function findMap(arr, f) { export { make , fromInitializer , - sort , indexOfOpt , lastIndexOfOpt , reduce , @@ -149,7 +133,6 @@ export { reduceRight , reduceRightWithIndex , findIndexOpt , - reverse , filterMap , keepSome , shuffle , diff --git a/src/Core__Array.res b/src/Core__Array.res index cd1bda48..03f25ff9 100644 --- a/src/Core__Array.res +++ b/src/Core__Array.res @@ -51,13 +51,17 @@ external copyWithin: (array<'a>, ~target: int, ~start: int, ~end: int) => array< @variadic @send external pushMany: (array<'a>, array<'a>) => unit = "push" -@send external reverseInPlace: array<'a> => unit = "reverse" +@send external reverse: array<'a> => unit = "reverse" +@send external toReversed: array<'a> => array<'a> = "toReversed" @send external shift: array<'a> => option<'a> = "shift" @variadic @send -external spliceInPlace: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = - "splice" +external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "splice" +@variadic @send +external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "toSpliced" + +@send external with: (array<'a>, int, 'a) => unit = "with" @send external unshift: (array<'a>, 'a) => unit = "unshift" @@ -92,13 +96,8 @@ let lastIndexOfOpt = (arr, item) => @send external sliceToEnd: (array<'a>, ~start: int) => array<'a> = "slice" @send external copy: array<'a> => array<'a> = "slice" -@send external sortInPlace: (array<'a>, ('a, 'a) => int) => unit = "sort" - -let sort = (arr, cmp) => { - let result = copy(arr) - sortInPlace(result, cmp) - result -} +@send external sort: (array<'a>, ('a, 'a) => int) => unit = "sort" +@send external toSorted: (array<'a>, ('a, 'a) => int) => array<'a> = "toSorted" @send external toString: array<'a> => string = "toString" @send external toLocaleString: array<'a> => string = "toLocaleString" @@ -154,15 +153,6 @@ let swapUnsafe = (xs, i, j) => { setUnsafe(xs, j, tmp) } -let reverse = xs => { - let len = length(xs) - let result = makeUninitializedUnsafe(len) - for i in 0 to len - 1 { - setUnsafe(result, i, getUnsafe(xs, len - 1 - i)) - } - result -} - let shuffleInPlace = xs => { let len = length(xs) for i in 0 to len - 1 { diff --git a/src/Core__Array.resi b/src/Core__Array.resi index 253f0cf2..cb6a12a0 100644 --- a/src/Core__Array.resi +++ b/src/Core__Array.resi @@ -165,7 +165,7 @@ Console.log(someArray) // ["hi", "hello", "yay", "wehoo"] external pushMany: (array<'a>, array<'a>) => unit = "push" /** -`reverseInPlace(array)` reverses the order of the items in the array. +`reverse(array)` reverses the order of the items in `array`. Beware this will *mutate* the array. @@ -174,13 +174,13 @@ See [`Array.reverse`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Re ## Examples ```rescript let someArray = ["hi", "hello"] -someArray->Array.reverseInPlace +someArray->Array.reverse Console.log(someArray) // ["hello", "h1"] ``` */ @send -external reverseInPlace: array<'a> => unit = "reverse" +external reverse: array<'a> => unit = "reverse" /** `shift(array)` removes the first item in the array, and returns it. @@ -201,22 +201,24 @@ Console.log(someArray) // ["hello"]. Notice first item is gone. external shift: array<'a> => option<'a> = "shift" /** -`sort(array, comparator)` returns a new, sorted array from `array`, using the provided `comparator` function. +`toSorted(array, comparator)` returns a new, sorted array from `array`, using the `comparator` function. -See [`Array.sort`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort) on MDN. +See [`Array.toSorted`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toSorted) on MDN. ## Examples ```rescript let someArray = [3, 2, 1] -let sortedArray = someArray->Array.sort((a, b) => a > b ? 1 : -1) +let sorted = someArray->Array.toSorted((a, b) => a - b) -Console.log(sortedArray) // [1, 2, 3] +Console.log(sorted) // [1, 2, 3] +Console.log(someArray) // [3, 2, 1]. Original unchanged ``` */ -let sort: (array<'a>, ('a, 'a) => int) => array<'a> +@send +external toSorted: (array<'a>, ('a, 'a) => int) => array<'a> = "toSorted" /** -`sortInPlace(array, comparator)` sorts `array` using the `comparator` function. +`sort(array, comparator)` sorts `array` in-place using the `comparator` function. Beware this will *mutate* the array. @@ -225,16 +227,21 @@ See [`Array.sort`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refer ## Examples ```rescript let someArray = [3, 2, 1] -someArray->Array.sortInPlace((a, b) => a > b ? 1 : -1) +someArray->Array.sort((a, b) => a - b) Console.log(someArray) // [1, 2, 3] ``` */ @send -external sortInPlace: (array<'a>, ('a, 'a) => int) => unit = "sort" +external sort: (array<'a>, ('a, 'a) => int) => unit = "sort" + +@variadic @send +external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "splice" + @variadic @send -external spliceInPlace: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = - "splice" +external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "toSpliced" + +@send external with: (array<'a>, int, 'a) => unit = "with" /** `unshift(array, item)` inserts a new item at the start of the array. @@ -805,7 +812,7 @@ Console.log(array[1]) // "Hello" external setUnsafe: (array<'a>, int, 'a) => unit = "%array_unsafe_set" /** -`findIndexOpt(array, checker)` returns the index of the first element of `array` where the provided `checker` function returns true. +`findIndexOpt(array, checker)` returns the index of the first element of `array` where the provided `checker` function returns true. Returns `None` if no item matches. @@ -826,19 +833,21 @@ switch array->Array.findIndexOpt(item => item == ReScript) { let findIndexOpt: (array<'a>, 'a => bool) => option /** -`reverse(array)` creates a new array with all items from `array` in reversed order. +`toReversed(array)` creates a new array with all items from `array` in reversed order. -See [`Array.reverse`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reverse) on MDN. +See [`Array.toReversed`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toReversed) on MDN. ## Examples ```rescript let someArray = ["hi", "hello"] -let reversed = someArray->Array.reverse +let reversed = someArray->Array.toReversed Console.log(reversed) // ["hello", "h1"] +Console.log(someArray) // ["h1", "hello"]. Original unchanged ``` */ -let reverse: array<'a> => array<'a> +@send +external toReversed: array<'a> => array<'a> = "toReversed" /** `get(array, index)` returns the element at `index` of `array`. diff --git a/src/Core__List.res b/src/Core__List.res index 27c248e7..317d3c2e 100644 --- a/src/Core__List.res +++ b/src/Core__List.res @@ -795,7 +795,7 @@ let setAssoc = (xs, x, k, eq) => setAssocU(xs, x, k, (. a, b) => eq(a, b)) let sort = (xs, cmp) => { let arr = toArray(xs) - Core__Array.sortInPlace(arr, cmp) + Core__Array.sort(arr, cmp) fromArray(arr) } diff --git a/src/typed-arrays/Core__TypedArray.res b/src/typed-arrays/Core__TypedArray.res index 343efc5e..208b982d 100644 --- a/src/typed-arrays/Core__TypedArray.res +++ b/src/typed-arrays/Core__TypedArray.res @@ -21,8 +21,13 @@ external copyWithin: (t<'a>, ~target: int, ~start: int, ~end: int) => array<'a> @send external fillInPlaceToEnd: (t<'a>, 'a, ~start: int) => t<'a> = "fill" @send external fillInPlace: (t<'a>, 'a, ~start: int, ~end: int) => t<'a> = "fill" -@send external reverseInPlace: t<'a> => t<'a> = "reverse" -@send external sortInPlace: (t<'a>, ('a, 'a) => int) => t<'a> = "sort" +@send external reverse: t<'a> => unit = "reverse" +@send external toReversed: t<'a> => t<'a> = "toReversed" + +@send external sort: (t<'a>, ('a, 'a) => int) => unit = "sort" +@send external toSorted: (t<'a>, ('a, 'a) => int) => t<'a> = "toSorted" + +@send external with: (t<'a>, int, 'a) => unit = "with" @send external includes: (t<'a>, 'a) => bool = "includes" From 89402ed8b778b9ba0c7330a769d04eb09c83a84a Mon Sep 17 00:00:00 2001 From: Cheng Lou Date: Mon, 22 May 2023 01:55:29 -0700 Subject: [PATCH 2/2] Fix types --- src/Core__Array.res | 5 +++-- src/Core__Array.resi | 5 +++-- src/typed-arrays/Core__TypedArray.res | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Core__Array.res b/src/Core__Array.res index 03f25ff9..6c447571 100644 --- a/src/Core__Array.res +++ b/src/Core__Array.res @@ -59,9 +59,10 @@ external copyWithin: (array<'a>, ~target: int, ~start: int, ~end: int) => array< @variadic @send external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "splice" @variadic @send -external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "toSpliced" +external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => array<'a> = + "toSpliced" -@send external with: (array<'a>, int, 'a) => unit = "with" +@send external with: (array<'a>, int, 'a) => array<'a> = "with" @send external unshift: (array<'a>, 'a) => unit = "unshift" diff --git a/src/Core__Array.resi b/src/Core__Array.resi index cb6a12a0..ada0d4f9 100644 --- a/src/Core__Array.resi +++ b/src/Core__Array.resi @@ -239,9 +239,10 @@ external sort: (array<'a>, ('a, 'a) => int) => unit = "sort" external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "splice" @variadic @send -external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => unit = "toSpliced" +external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => array<'a> = + "toSpliced" -@send external with: (array<'a>, int, 'a) => unit = "with" +@send external with: (array<'a>, int, 'a) => array<'a> = "with" /** `unshift(array, item)` inserts a new item at the start of the array. diff --git a/src/typed-arrays/Core__TypedArray.res b/src/typed-arrays/Core__TypedArray.res index 208b982d..16714279 100644 --- a/src/typed-arrays/Core__TypedArray.res +++ b/src/typed-arrays/Core__TypedArray.res @@ -27,7 +27,7 @@ external copyWithin: (t<'a>, ~target: int, ~start: int, ~end: int) => array<'a> @send external sort: (t<'a>, ('a, 'a) => int) => unit = "sort" @send external toSorted: (t<'a>, ('a, 'a) => int) => t<'a> = "toSorted" -@send external with: (t<'a>, int, 'a) => unit = "with" +@send external with: (t<'a>, int, 'a) => t<'a> = "with" @send external includes: (t<'a>, 'a) => bool = "includes"