Skip to content

ImmutableArray #152

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

Closed
wants to merge 5 commits into from
Closed

Conversation

jderochervlk
Copy link
Contributor

@jderochervlk jderochervlk commented Jul 28, 2023

An addition to add an ImmutableArray type to Core.

Conversation over here: #23

This just omits any functions that would mutate the original Array and adds a function to create an ImmutableArray from an Array.

Here's an example of how to create one:

let a1 = [1, 2, 3]

let immutableA1 = RescriptCore.ImmurableArray.fromArray(a1)

And here's the output:

// Generated by ReScript, PLEASE EDIT WITH CARE


var a1 = [
  1,
  2,
  3
];

var immutableA1 = a1.slice();

export {
  a1 ,
  immutableA1 ,
}
/* immutableA1 Not a pure module */

I made fromArray use slice to create a copy of the original array. This means that once we create an ImmutableArray from a normal Array we don't have to worry about the original array mutating. Of course we only have that guarantee that no one will mutate the array if all of our source code is written in Rescript.

I'll address these items if this gets the go ahead to be added.
TODO:

  • update and write docs
  • write tests

@jderochervlk jderochervlk changed the title feat: ImmutableArray ImmutableArray Jul 28, 2023
@jmagaram
Copy link
Contributor

JavaScript and .net combine both mutable and immutable array functions in the same module because it is convenient for the programmer and has performance benefits. The programmer can use immutable functions most of the time and carefully switch over to a mutable "push" for performance benefits. I think arrays are mutable almost by definition. I like immutability, but just don't think Array is a natural fit for it.

If we have such a module that represents data structures NOT in JavaScript I think other data structures are higher priority or more useful. For example the Belt Set and Map allow you to define the item equality operator, without which the JavaScript Map and Set are somewhat useless. Here are immutable collections for .net - lots of interesting ones to choose from. https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable?view=net-7.0 Personally I find a NonEmptyArray to be useful. And finally I think lazy sequences are extremely useful, especially for working with arrays, are immutable, and are part of most mature frameworks. I think sequences are far higher priority than immutable arrays. https://github.com/jmagaram/rescript-seq
So if we're going to have immutable arrays I think this should be part of a larger discussion about data structures.

As for this particular code, once you've got an array it seems like you'd need a toArray so you could consume it with most of the code out there that requires one. I would expect this to be zero cost and introduce the risk that some other code will change it.

@jderochervlk
Copy link
Contributor Author

jderochervlk commented Aug 2, 2023

As for this particular code, once you've got an array it seems like you'd need a toArray so you could consume it with most of the code out there that requires one.

Correct. This would mean you would have to convert this to a regular Array before you could pass it to React.array for example. This type of moving around can be a pain, but it's no different than using Immutable.js where that type of workflow is common.

Ideally this would be similar to the ReadonlyArray type from TypeScript which doesn't allow the functions that are removed in this PR. You also can't pass a ReadonlyArray to a function that expects an array, but in TS you can also take in a: ReadonlyArray<int> | Array<int>.

https://www.typescriptlang.org/play?#code/DYUwLgBGCMBcECUQEMAmB7AdsAngQQCcDkcAeTAVwFsAjEAgPggF4IBtaAGgCZOBmALoAoIQDMKmAMZgAllggypBEFRCYwACmTxCxMpVr0GASggBvIRCsRlYCgUwRkAOirIADhsfMmjgNQQ0MZCAL4ioJDKAM4UwJCsipLKquoaMMFiEtJyjqKgAB4yNKAAkkoqapraELok5NR0jBAAPogoGNj4RHUGjSbmlta29o4ubp7evhABQaHh4BBRIJJYqEgxcSwQeSCFxSBlSRWp6UA

The programmer can use immutable functions most of the time and carefully switch over to a mutable "push" for performance benefits. I think arrays are mutable almost by definition. I like immutability, but just don't think Array is a natural fit for it.

This would behave similar to a ReadonlyArray in TS, which is a feature well known to TS devs. It would be really nice to have that same concept in Rescript's Core library.

What I would like is something that maps to a JS Array under the hood but doesn't allow mutation, and has some type of guarantee from the compiler that the array hasn't been mutated by something else.

If I do need to interop with a regular array the need to transform from immutable and back again means that even if the function I am calling mutates the array it would only be mutating a copy.

Here's another example:

let t1: ImmutableArray.t<int> = [1, 2, 3]->ImmutableArray.fromArray

let t2 = t1->ImmutableArray.toArray

let t3 = t2->Array.push(4)

Console.log2(t1, t2) // [1, 2, 3], [1, 2, 3, 4]

My original array of t1 cannot be changed by the push, which is the desired behavior for an ImmutableArray.

Would it make sense to call this a ReadonlyArray to match TypeScript?

@cristianoc
Copy link
Contributor

Notice casting one type of array to the other (either direction) without copying is unsafe.
Also, immutable array can be +'a so it works with sub typing.

@cristianoc
Copy link
Contributor

In case one wants to intentionally be unsafe (immutable arrays can actually be mutated by code that type checks), copying TS behavior, one would need to explain the benefits of the choice.

@jderochervlk
Copy link
Contributor Author

Notice casting one type of array to the other (either direction) without copying is unsafe.
Also, immutable array can be +'a so it works with sub typing.

Correct, in order to move from a mutable array from immutable, or the other way around, the array is copied.

In case one wants to intentionally be unsafe (immutable arrays can actually be mutated by code that type checks), copying TS behavior, one would need to explain the benefits of the choice.

TypeScript does it's best to try and stop you from mutating a readonly array.

You can't do this for example:

function fn(a: Array<number> | ReadonlyArray<number>) {
    return a.push(1) // Property 'push' does not exist on type 'readonly number[]
}

TypeScript also doesn't let you swap the type from a ReadonlyArray to an Array, you need to make a copy.

function convert<T>(a: ReadonlyArray<T>): Array<T> {
    return [...a]
}
function convert<T>(a: ReadonlyArray<T>): Array<T> {
    return a //The type 'readonly T[]' is 'readonly' and cannot be assigned to the mutable type 'T[]'.
}

@jderochervlk
Copy link
Contributor Author

jderochervlk commented Aug 3, 2023

Yeah, TypeScript isn't perfect. Hopefully this is something that can be enforced going in both directions with ReScript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants