Refining to an array no longer allows me to edit the array using splice

[romeovs]

Missing/Incorrect APIs

Before (last version I checked is 0.92.1), it was possible to refine mixed to an array using Array.isArray and edit the array using splice:

const arr : mixed = foo()

if (Array.isArray(x)) {
  const s = arr.splice(0, 3)
}

Before, arr was an Array<mixed> and this would work. But since 5639532065954e7f66a968af5e68f76a762a9c14, this will no longer work since arr will be a $ReadOnlyArray.

I still think my code should be valid though, since there's no writing of values into the array so I'm not changing the type (which is what 5639532065954e7f66a968af5e68f76a762a9c14 was designed to tackle).

Maybe there needs to be an intermediate array type that does not allow writing values but does allow for slicing up the array

If not, what would be a good way to work around this? I'm writing code that works with arrays without caring about the values in the array, but that needs to be able to split up the arrays.

[goodmind]

Isn't splice mutating method?

[romeovs]

@goodmind Yes it is, so I understand it can not be part of $ReadOnlyArray, but the issue here is that refining mixed to an array of mixed is creating a $ReadOnlyArray<mixed>, which prevents some valid programs (like mine) to be type checked.

The reason $ReadOnlyArray<mixed> was chosen in 5639532 is because Flow needs to make sure the type of the underlying array cannot be changed or invalid programs can be written (see the example in 5639532).

My point is that $ReadOnlyArray<mixed> is too strict to reach that goal, because it misses the slice method which, as I show in my example above, also leaves the type of underlying array untouched.

[lyleunderwood]

I think you're probably right in this case that there would need to be an intermediate array type to model this problem, one where you can maybe move elements around and delete them but not add anything.

There's a second problem though, which is with splice. In theory you can use splice to add elements to an array which would be writing its contents and potentially changing its type. I don't think you can currently target a specific class based on function arity with flow, but I think that's what you would want to do, basically break up the splice definition based on arity. If you did introduce a new intermediate array type, splice would still require a full Array I think unless you could target its implementation based on arity.

[goodmind]

So it would be something like this?

declare class $PermutableArray<+T> extends $ReadOnlyArray<T> {
  copyWithin(target: number, start: number, end?: number): Array<T>;
  every(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => any,
    thisArg?: any
  ): boolean;
  filter(callbackfn: typeof Boolean): Array<$NonMaybeType<T>>;
  filter(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => any,
    thisArg?: any
  ): Array<T>;
  find(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => any,
    thisArg?: any
  ): T | void;
  findIndex(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => any,
    thisArg?: any
  ): number;
  forEach(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => any,
    thisArg?: any
  ): void;
  map<U>(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => U,
    thisArg?: any
  ): Array<U>;
  pop(): T;
  reduce(
    callbackfn: (
      previousValue: T,
      currentValue: T,
      currentIndex: number,
      array: $PermutableArray<T>
    ) => T,
    initialValue: void
  ): T;
  reduce<U>(
    callbackfn: (
      previousValue: U,
      currentValue: T,
      currentIndex: number,
      array: $PermutableArray<T>
    ) => U,
    initialValue: U
  ): U;
  reduceRight(
    callbackfn: (
      previousValue: T,
      currentValue: T,
      currentIndex: number,
      array: $PermutableArray<T>
    ) => T,
    initialValue: void
  ): T;
  reduceRight<U>(
    callbackfn: (
      previousValue: U,
      currentValue: T,
      currentIndex: number,
      array: $PermutableArray<T>
    ) => U,
    initialValue: U
  ): U;
  reverse(): Array<T>;
  shift(): T;
  some(
    callbackfn: (value: T, index: number, array: $PermutableArray<T>) => any,
    thisArg?: any
  ): boolean;
  sort(compareFn?: (a: T, b: T) => number): Array<T>;
  splice(start: number, deleteCount?: number): Array<T>;

  +[key: number]: T;
  length: number;
}

[lyleunderwood]

Yeah I think so. But @goodmind shouldn't sort and splice return $PermutableArray<T> also since they don't return a new array? Returning Array<T> here would allow the element type of the original array to change, I think.

@romeovs if I were you I would just pull the above type into your project and then cast your $ReadOnlyArray to this $PermutableArray and see how that works out for you, like:

const arr : mixed = foo()

if (Array.isArray(arr)) {
  ((arr: any): $PermutableArray<mixed>);
  const s = arr.splice(0, 3)
}

or maybe wrap it in a helper:

const withPermutation = <+T>(arr: $ReadOnlyArray<T>): $PermutableArray<T> =>
  ((arr: any): $PermutableArray<T>);

(Try Flow)

Actually getting something like this into flow would I think mean changing array refinement to return a $PermutableArray rather than a $ReadOnlyArray which might be a breaking change that would introduce all kinds of subtle errors to existing code. I'm not sure how big a deal it would be, because a $PermutableArray can, of course, cast directly to a $ReadOnlyArray. I don't know enough about how classes work in flow, I guess. Either way, I'm sure adding another basic array type would have pretty wide-ranging implications.

[goodmind]

/cc @dsainati1

[eilvelia]

changing array refinement to return a $PermutableArray

Then any $ReadOnlyArray can be cast to $PermutableArray, would be that safe?

declare var ra: $ReadOnlyArray<string>
const ram: mixed = ra
if (Array.isArray(ram)) {
  const pa: $PermutableArray<mixed> = ram
  pa.sort()
}