Upgrade to ReadOnlyArray (TS 2.0)

[jonaskello]

In TypeScript 2.0 we have ReadOnlyArray which I think is a perfact match for a library like ramda that does not do any mutations.

Would it be OK if I did a PR that changes to ReadOnlyArray?

Here is a short example how it works:

let a: Array<number> = [0, 1, 2, 3, 4];
let b: ReadonlyArray<number> = a;
b[5] = 5;      // Error, elements are read-only
b.push(5);     // Error, no push method (because it mutates array)
b.length = 3;  // Error, length is read-only
a = b;         // Error, mutating methods are missing

[KiaraGrouwstra]

I respect the pure FP views here, though it'd make for a breaking change. Certainly, Ramda does not aim to aid in mutating objects. Technically though, its results though are still JS objects, and technically allow the user to mutate them otherwise as they wish.

I'd compare this to the views at Ramda itself, which have been somewhat careful, relegating the 'breaking' pure-FP views to the likes of ramda-fantasy and sanctuary so as to ensure different audiences can be catered to.

My inclination here would be to follow that example, and welcome this approach as a separate branch for those who appreciate the limitation it imposes.
That said, perhaps @donnut could chime in here as well.

[KiaraGrouwstra]

I just gave this a second thought. I realize just relegating this to a branch would be unfortunate in the sense it'd make for duplicate work on further edits, and I was thinking of a way we could potentially address this.

I'd propose having a PR to add e.g. type OutputArray<T> = Array<T>; in master branch, then wrapping any arrays in results using that rather than using Array / [] directly. That way, a branch for ReadOnlyArray would only require making one actual change, and edits from master branch could be directly merged into it.

I realize it remains a compromise, but might this perhaps work out for your purposes @jonaskello?

[jonaskello]

Yes, seems like a nice solution! For input parameters we could directly use ReadonlyArray since it will not be a breaking change (because of structural typing you can pass regular Array to ReadonlyArray). But for output arrays as you notice, it would be a breaking change. So having a type alias OutputArray like you propose is a nice compromise to keep it non-breaking and easy to change to fully immutable. I'll post a PR with these changes.

[KiaraGrouwstra]

Sounds good. Thanks! :)

[KiaraGrouwstra]

Related: given the request for ImmutableJS support, I wonder if for arrays in input params we should perhaps just use Iterable instead. In that event, I suppose that way ReadOnlyArray would get supported there as well.

[jonaskello]

I think using Iterable would require some more consideration than I am currently doing. For example adding more tests. So I think it might be better to do it in two steps. I could make InputArray a type alias too like type InputArray<T> = Array<T> and later we could easily change to type InputArray<T> = Iterable<T>. Or you can do a text search for InputArray and change it directly to Iterable if that is desired. As it is right now everything is declared as T[] and I am going through them and classifying as Input or Output.

[KiaraGrouwstra]

I'm currently handling the input parameters for that, testing on a case-by-case basis with Ramda. It appears Iterable isn't actually supported for most functions, while ArrayLike (handles Array, ReadOnlyArray, string) appears fine. I was currently adding in ArrayLike there, but using that through a variable would work for me as well. Output ones I haven't touched.

…

On Fri, Dec 9, 2016 at 4:45 PM, Jonas Kello ***@***.***> wrote: I think using Iterable would require some more consideration than I am currently doing. For example adding more tests. So I think it might be better to do it in two steps. I could make InputArray a type alias too like type InputArray<T> = Array<T> and later we could easily change to type InputArray<T> = Iterable<T>. Or you can do a text search for InputArray and change it directly to Iterable if that is desired. As it is right now everything is declared as T[] and I am going through them and classifying as Input or Output. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#113 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC6uxceTi23bRB2voI-QYdOL2wsN_F_Mks5rGRUTgaJpZM4K9tNW> .

[KiaraGrouwstra]

I just pushed a switch to ArrayLike for input parameters. Please try a pull -- I hope I haven't messed too much with your current efforts!
On an InputArray, I'm currently wondering if there'd be an immediate use-case for generalizing this part. Nevertheless, if you'd already been doing that, I'm open to the idea. That said, do note I've already sorta tested which parts can use ArrayLike (and which can use Iterable -- found none).

[jonaskello]

I tried to merge my fork but got conflicts on every line :-). It seems like all input arrays are changed to List<T>. I think I need to re-fork but before I do I think it might be a good idea that we have a plan on what should be done. I'll do some checking on the new code and see if it covers my use-cases.

[jonaskello]

Ok, so I think I see now where you are going with this. This is the definition of ArrayLike<T>:

interface ArrayLike<T> {
    readonly length: number;
    readonly [n: number]: T;
}

And you are saying that you have checked all functions in Ramda and the only thing they ever do is check length or query by index? In that case this is a nice solution because it would cover ReadonlyArray too.

[KiaraGrouwstra]

Sorry about that :(, hope you've been able to fix that with a mass-replace. I'll admit I've been editing... a bunch. I noticed Lodash was using this List abstraction, and figured it was kinda nice for its brevity.

On ArrayLike, that's correct. The thing about Ramda is, it appears to be doing good old-fashioned for-loops to iterate over lists, and it just so happened this only required those length and numerical attributes.
This is also the reason everywhere you can input a string[] you can also just input a string (it'll loop over it and grab the separate characters), and why I imagine other practically-arrays like ReadOnlyArray and ImmutableJS's List should all work by default as well. So I suppose at that point it's largely a matter of those output arrays...

On that topic, I might want to inquire how branches might work at DefinitelyTyped...

[jonaskello]

No problem :-). I actually think I don't need to add anything to your edits for ReadonlyArray to work as input now that ArrayLike is in place. So I guess we could just close this?

Btw my use-case for ReadonlyArray is that I want to use these tslint rules instead of ImmutableJS or seamless-immutable.

[jonaskello]

IIRC it is not possible to create your own indexers in JS, it is only regular objects or arrays that are indexable with the [] operator. So I think ImmutableJS it uses special .get and .set methods which would make them incompatible with ArrayLike. But that is not important for this issue :-).

[KiaraGrouwstra]

Okay, if you turn out to need more let's reopen then. :)
The tslint rule-set sounds interesting. Does that automatically make any array immutable? Or would you specifically have to start using ReadOnlyArrays yourself?

On Immutable, we just had another thread as well, but yeah, I definitely feel Ramda is being a bit old-fashioned there by doing loops instead of hooking into the Iterable API, but yeah, perhaps they hadn't really considered alternative input structures like Set or ImmutableJS Lists so far... Could have been performance considerations for all I know.

Then again, it might raise questions on whether in that case return types should use Set or List as well.
Come to think of it, I think it should be doable to inspect an input Iterable's constructor and construct a return value using that. Just might raise questions for functions like concat in the event the two inputs would use different Iterables. But that seems an implementation detail.

[KiaraGrouwstra]

Correction: seems they do plan on doing iterators.

[rajdeep26]

Is this issue resolved? How can i use ReadonlyArray<T> with forEach function? Doesn't seem to work for me.

[KiaraGrouwstra]

Hm. Sounds weird, List should still support ArrayLike... how did you install this @rajdeep26 (package/version)?

[rajdeep26]

@tycho01 the types for ramda is installed using yarn add @types/ramda.
The version is "@types/ramda": "^0.24.11"

BTW With this version, is this suppose to work?

const arr: ReadonlyArray<number> = [1,2,3,4,5]
// assuming fn is some function defined somewhere
Ramda.forEach(fn, arr);

I still get this error:

severity: 'Error'
message: 'Argument of type 'ReadonlyArray<number>' is not assignable to parameter of type 'number[]'.
  Property 'push' is missing in type 'ReadonlyArray<number>'.'

[KiaraGrouwstra]

@rajdeep26: thanks for that. Would you mind trying yarn add --dev types/npm-ramda#dist? We haven't managed to update the one on DT (@types/ramda) yet.