Array.last: add support for ReadonlyArray

[OliverJAsh]

Currently this function will error when trying to pass in an array of type ReadonlyArray<T>.

I suspect we'll want to make the same change for other functions which receive arrays… but it does seem like a PITA.

[gcanti]

Array<A> is a subtype of ReadonlyArray<A> so I guess that its signature could simply be

<A>(as: ReadonlyArray<A>): Option<A>

Now the question is: should we support ReadonlyArrays? What about functions which return arrays like copy or sort?

@sledorze @raveclassic what do you think? What's your experience with the Readonly* variants?

p.s.
Personally I never bothered to use them, nor the readonly modifier (except when I started using TypeScript). Actually after 19 months of TypeScript daily usage I never experienced a bug related to mutations, I just don't mutate any data structure.

[OliverJAsh]

For context, I'm receiving a ReadonlyArray from io-ts’ ValidationError['context']: https://github.com/OliverJAsh/io-ts-reporters/blob/27314683a28b6798c2a357e41477786f4755febc/src/index.ts#L22

[gcanti]

Indeed, theoretically using ReadonlyArray is the "correct" thing to do in that case, in practice I'm not sure it was a good idea.

p.s.
Btw in that case Context should be a NonEmptyArray<ContextEntry> so that picking the last element is a total function (I plan to refactor the validation errors in io-ts@2.0.0)

[sledorze]

I never use ReadOnlyArray but when a lib exposes it. (we also never mutate but in lowlevel functions with care of keeping referential transparency)

The general reason tu support ReadOnlyArray is to integrate with third part lib.
A discussed here:
typed-typings/npm-ramda#113

Note: I'm interested into knowing more about the refactor of errors goal in io-ts

[gcanti]

should we support ReadonlyArrays?

Proposal 1:

if Array does appear in negative position, switch to ReadonlyArray

+export const last = <A>(as: ReadonlyArray<A>): Option<A> => {
-export const last = <A>(as: Array<A>): Option<A> => {

otherwise add an overloading

+export const copy = <A>(as: Array<A>): Array<A>
+export const copy = <A>(as: ReadonlyArray<A>): ReadonlyArray<A>
+export const copy = <A>(as: ReadonlyArray<A>): ReadonlyArray<A> => {
-export const copy = <A>(as: Array<A>): Array<A> => {

However I'm not sure how to deal with something like: getMonoid, getSetoid, getOrd, etc...

export const getMonoid = <A = never>(): Monoid<Array<A>> => {
  ...
}

or something like range

export const range = (start: number, end: number): Array<number> => {
  ...
}

Proposal 2:

Add a ReadonlyArray module

/cc @RoryDungan

[RoryDungan]

@gcanti mentioned previously that, while Array contains all properties of ReadonlyArray so changing function arguments to take ReadonlyArrays wouldn't break backwards compatibility, it would cause an issue with if we changed existing functions to return ReadonlyArrays.

The solution he proposed was adding overloaded versions of each array function that handle ReadonlyArrays, but another option I think could be worth considering would be to change existing functions that take Arrays as arguments to take ReadonlyArrays, but leave them to return regular Arrays.

As far as I can tell all fp-ts array functions return a new array separate from the source one so having them be different types shouldn't cause an issue. Since Array is a superset of ReadonlyArray, functions of type ReadonlyArray -> Array can still be composed as if they were of type Array -> Array.

[gabejohnson]

I'm working on an application that uses https://github.com/jonaskello/tslint-immutable to keep the other devs on the team honest. One of the rules (which I want enforced) is use of ReadonlyArray.

I frequently find myself having to cast them to Array when I'm using this library. I was honestly surprised that the array functions didn't return ReadonlyArray as there is a strong tendency away from mutation.

[cruhl]

This has also been an issue for me.

[gcanti]

@gabejohnson curious about tslint-immutable, in your experience is there any difference in practice between readonly-array and no-array-mutation rules?

• readonly-array
• no-array-mutation

[gabejohnson]

@gcanti no-array-mutation wasn't added until after we began the project (and I wasn't aware of it until today). We swapped it in and it works great!

[gcanti]

Closing due to inactivity