[Discussion] Immutable defaults for mutable JS API:s #23
Comments
|
Also, there's https://github.com/tc39/proposal-change-array-by-copy. The question is how far we should go from the JS API |
|
From perspective of a ReScript module API, making dangerous functions more visible is good, but I’m not sure if it’s good enough. When a module But when you simply discourage mutating, say, arrays, instead of completely ruling it out, you don’t have those guarantees. In a team of app developers that have other things on their mind than just remembering not to mutate things, you’d probably at least want a linter to strongly discourage calling That said, it’s rather weird to omit bindings when binding to
I’m torn on this. I like the idea of clearly separating getters and setters, but then bindings like this are not true to the underlying API, which might be too opinionated for a standard JS lib, and I bet some people are going to miss the chained calls those return values enable. |
I totally agree going on this direction, for me it's more important to be able to use the real JS api, and be consistent with the real JS api, than having some pure immutable layer, and for several reasons including performance and porting existing JS code.
This makes sense even though there are some benefits like chaining, it's clearer to leave mutable operations as
I would prefer to be consistent with JS api if one really cares about the JS ecosystem, it may be consistent with rescript ecosystem, but not with JS. |
|
Fo example Scala.js has a set of both mutable and immutable collections, while also keeping the |
Yeah this pretty much sums it up well I think. We're trying to stay close to the JS array API, which is ultimately what we're binding to, not just as a backing structure. An immutable array module is of course very welcome in |
|
I haven't really made up my mind yet, but I want to share where I'm coming from and what my thoughts are to facilitate further discussion: My hope / expectations for a new rescript standard library has been a "full-fledged belt-like" lib embracing functional idioms. I understand this is a non-goal of Core and absolutely see the necessity to have all JS bindings in one place and well documented. Regarding immutability:
I'm aware this is becoming slightly off-topic, but I think the implications are pretty relevant to the discussion. |
|
@woeps I'm guessing you saw this https://github.com/rescript-association/rescript-core#guiding-principles? |
|
hoichi has a good point ‘When a module Foo has no mutating methods, you have a strong guarantee that Foo.t is immutable’ What about keeping the api very close to the original js (with a few small tweaks to remove warts etc.. eg returning unit for mutation) and separate immutable datatypes eg as per tc39 proposal. https://github.com/tc39/proposal-record-tuple |
|
Yes, that's the best way forward here. |
|
I would be down to try and implement this. I have some questions I would like feedback on first. Where does this go?Should it be in My thoughts are we could have What should
|
|
Technically, it's possible to have a single set of bindings that map closely to the JS API, and also have a subset of them work on a distinct immutable array type, by adding a phantom type argument. // Instead of
type array<'a>
// we add a phantom type argument to specify mutability
type array<'mut, 'a>
// then a couple of abstract phantom types to put there
type mut
type imm
// and now you can define functions that work on every kind of array, or just one kind
@get_index external get: (array<_, 'a>, int) => option<'a> = "" // can be called on any kind of array
@set_index external set: (array<mut, 'a>, int, 'a) => unit = "" // can only be called on mutable arraysNot saying this is a good idea, there are certainly significant downsides to it, like making the types and type error messages more complicated, but it does make for a smaller API surface and is quite intuitive I think. |
|
Wouldn't that mean that while my call of |
|
Not sure what you mean. |
|
Sorry I wasn't clear. If you have an I think there should be a clear distinction between an array and an immutable array. |
The issue with that is going to be variance. |
|
As for where this should live, there was talks a while ago about a separate package dedicated to ReScript data structures, where this and several other things would fit well. Might be good to start with that, and then we can decide on where it should live more long term when it's tried and tested. |
That is is exactly what I'm proposing. There would be no |
Can you illustrate the issue with an example? If I understand correctly, covariance would just mean that if |
Yes that's what it means. |
Would it be a good idea for me to start a repo and publish some type of alpha release for this? I could do |
|
Nevermind, I just needed a |
You can't convert a mutable array to an immutable one without making a copy. It's unsafe. |
|
Same for the other way round. |
I did not really answer your question: why it matters. That's just one example. Another one: how comes I can't type coerce array of this record to array of that record? (Question people asked). Because arrays are not covariant. |
|
Ok, I based it off from the work done here in this thread: https://forum.rescript-lang.org/t/immutable-array-implementation/761 module Array: {
/* Put this in the interface file */
type t<'a>
let make: array<'a> => t<'a>
let isEmpty: t<'a> => bool
let head: t<'a> => option<'a>
let tail: t<'a> => t<'a>
let toArray: t<'a> => array<'a>
let map: (t<'a>, 'a => 'b) => t<'b>
let concat: (t<'a>, 'a) => t<'a>
let append: (t<'a>, 'a) => t<'a>
let get: (t<'a>, int) => option<'a>
let set: (t<'a>, int, 'a) => t<'a>
} = {
/* Put this in the module file */
type t<'a> = array<'a>
let make = arr => arr->Array.copy
let isEmpty = t => t->Array.length == 0
let head = t => t->Array.get(0)
let tail = t => isEmpty(t) ? [] : t->Array.slice(~start = 1, ~end = t->Array.length)
let toArray = make
let map = Array.map
let concat = (t, a) => [a]->Array.concat(t)
let append = (t, a) => t->Array.concat([a])
let get = (t, i) => t->Array.get(i)
let set = (t, i, v) => {
let c = t->Array.copy
c->Array.set(i, v)
c
}
}let t1 = [1,2,3]
let t2 = t1->Immutable.Array.make->Immutable.Array.head // no error
let t3 = t1->Immutable.Array.head // errorI'm going to clean this up and add more functions and some basic documentation. |
|
The way I have it above it's not a zero cost binding. Trying to see if I can do that now. |
|
The more I poke at this I don't think there is a way to have this pass directly to JS functions directly. There has to be a middle layer for these types to work and to not mutate the input values. But that's something I might be able to improve on. |
On needs to decide what immutable means. |
|
To me I would expect it not to mutate and to return a copy. The first case of not mutating can be done just simply not using the functions that could be omitted. |
Thanks for the examples! These problems aren't going to go away by adding a separate second-class immutable array type though. As long as the default is a mutable array, and that is what's used by all sorts of bindings and such, people are still going to stumble across these issues and get confused. And at that point, are these issues severe enough that they'll choose to switch over to an immutable array, converting to and from mutable arrays where needed and adapt to different usage patterns? |
I don't think there are easy answers here either way. |
That is a good point. Users would have to start with an mutable array, convert it to immutable, and then convert back to a mutable one. That is a pain, but it is something users already have to do when working with Immutable.js. Implementation aside, what I think would be a good experience for the user would be the ability to create an Immutable array that is somehow still an |
If it's an The only way to avoid that, is to store a mutable bit with the array. And fail at runtime if you try to mutate it. Not sure how useful that would be. |
I'd rather like to have a compile error, than a runtime error. If I want to mutate an ImmutableArray I need to convert it to an array first. (implemented as copying the array and returning it typed as an array). This seems fine, since I'd need to do this anyway for any other datastructure. |
Yeah, the more I think about it the more I like the idea of a separate type for |
|
Seems like what we really want is linear types, like Rust has. Though that wouldn't solve the variance issue either... |
Starting to work on this concept: rescript-lang/rescript-core#23
I went ahead and tested this idea out. For now I created a different type called // a.res
module Array: {
type _array<'mut, 'a>
type mut
type imm
@get_index external get: (_array<_, 'a>, int) => option<'a> = "" // can be called on any kind of array
@set_index external set: (_array<mut, 'a>, int, 'a) => unit = "" // can only be called on mutable arrays
@send
external make: array<'a> => _array<mut, 'a> = "slice"
@send
external fromImmutable: array<'a> => _array<mut, 'a> = "slice"
module Immutable: {
@send
external make: array<'a> => _array<mut, 'a> = "slice"
let set: (_array<_, 'a>, int, 'a) => _array<imm, 'a>
}
} = {
type _array<'mut, 'a> = array<'a>
type mut
type imm
@get_index external get: (_array<_, 'a>, int) => option<'a> = "" // can be called on any kind of array
@set_index external set: (_array<mut, 'a>, int, 'a) => unit = "" // can only be called on mutable arrays
@send
external make: array<'a> => _array<mut, 'a> = "slice"
@send
external fromImmutable: array<'a> => _array<mut, 'a> = "slice"
module Immutable = {
@send
external make: array<'a> => _array<mut, 'a> = "slice"
let set = (a, i, v) => {
let c = a->Array.copy
c->Array.set(i, v)
c
}
}
}index.res
let t1 = [1, 2, 3]->A.Array.make
let t2 = t1->A.Array.get(0)
let t3 = t1->A.Array.set(0, 100)
let t4 = t1->A.Array.Immutable.set(0, 10) // array is now immutable
let i1 = [1, 2, 3]->A.Array.Immutable.make
let i2 = i1->A.Array.get(0)
// let i3 = i1->A.Array.set(10, "foo") // error
let i4 = i1->A.Array.Immutable.set(0, 10)// a.mjs
// Generated by ReScript, PLEASE EDIT WITH CARE
function set(a, i, v) {
var c = a.slice();
c[i] = v;
return c;
}
var Immutable = {
set: set
};
var $$Array = {
Immutable: Immutable
};
export {
$$Array ,
}
/* No side effect */
// index.js
// Generated by ReScript, PLEASE EDIT WITH CARE
import * as A from "./a.mjs";
import * as Curry from "rescript/lib/es6/curry.js";
var t1 = [
1,
2,
3
].slice();
var t2 = t1[0];
t1[0] = 100;
var t4 = Curry._3(A.$$Array.Immutable.set, t1, 0, 10);
var i1 = [
1,
2,
3
].slice();
var i2 = i1[0];
var i4 = Curry._3(A.$$Array.Immutable.set, i1, 0, 10);
var t3;
export {
t1 ,
t2 ,
t3 ,
t4 ,
i1 ,
i2 ,
i4 ,
}
/* t1 Not a pure module */It feels easier to use than the separate |
|
I was wondering where immutable data structures arise as a fundamental need. So I thought that if such a need exists, it would show up in games. |
|
I think that need would mostly arise from wanting to use immutable arrays, and needing to interop with JS APIs which would expect (or are at least most safely typed as) mutable arrays.
The module Array: {
type t<'mut, 'a>
}
type array<'a> = Array.t<mut, 'a>It's a pretty simple code mod if it is made a breaking change too though. |
|
I think the simplest possible solution already teaches the top complexity budget justified by the need for this. |
|
While I think having Does this mean we should continue to look at using a separate implementation for immutable arrays, like There is also the option to just recommend Here's an example: let t1 = [1,2,3]->List.fromArray->List.get(0)
/*
// compiles to
var t1 = Core__List.get(Core__List.fromArray([
1,
2,
3
]), 0);
*/
let t2 = [1,2,3]->Array.get(0)
// compiles to
// var t2 = 1 |
|
I opened up a PR to add an |
Following the discussion about immutable defaults in https://forum.rescript-lang.org/t/ann-rescript-core-a-new-batteries-included-drop-in-standard-library/4149, let's discuss which API:s are problematic and what we can do to remedy that.
First off, there's a few things we need to keep in mind in this discussion:
Core. Not everyone is going to agree with this, and that's fine. But that's the way forward forCore.Beltis still a great alternative, andBeltisn't going anywhere, it'll keep being around.With that said, providing immutable defaults where it makes sense is definitively a good idea. So, let's detail what API:s are problematic right now. Please write down the API:s with mutability you worry about, and we'll discuss what we can do about it, if anything.
Let's also discuss what we can do about the mutable API:s to make it apparent that they're mutable. A few things come to mind:
unitalways, even if the underlying JS API returns itself mutated. One example for this isMap.setwhich mutates and returns itself. This would returnunitinstead to signify that it's mutating.inPlace. We can check that that's consistent.**This mutates the original array**on top of each doc string, plus suggesting the immutable alternative. This will make it very apparent that you're using a mutable API as you're working in your editor.The text was updated successfully, but these errors were encountered: