adamhaile/S-array

SArray does not extends Array<T>

michal-minich opened this issue · 2 comments

from index.d.ts:

export interface SArray<T> {... }
export interface SDataArray<T> extends SArray<T>

I would proffered this be written as:

export interface SArray<T> extends ReadonlyArray<T> { 
   // only S-Array specific function mentioned here
}
export interface SDataArray<T> extends SArray<T>, Array<T> { 
   // only S-Array specific function mentioned here
}

Obviously the implication are

  1. The additional function and their overloads presented in Readonly/Array would need to be implemented in S-Array.
  2. Existing functions with different signatures would need to be changed or overloads added
    • I experienced this issues with .map function, when used with more than 1 argument.

The reason is this would allow general (generic) function to be written that can handled both types. Also converting code to-and-from standard array / s-array.

What is your opinion?

Thank you.

This comes down to a similar issue to the one I made in your question about .length. S signals aren't meant to act like a value, they're meant to carry a value. SArray does cut against this a bit, in that it mimics the standard array methods to provide familiar convenience methods for working with signals carrying arrays. But it's not a goal (can't be really) to be 100% interchangeable with a raw array.

You advertise the compatibility on the main page. And I think this is a good thing!

[...]These utility methods parallel the standard ES3/5/6 array methods.

I really don't think it is necessary to support all Array methods. But I think it is important the ones that are present have the same signature and behavior. Otherwise it is confusing and makes translating code unnecessarily error-prone.

Current issues I know of:

  • .map has different signature for 2+ arguments

  • .length has different behavior

  • I think it is not necessary to SArray to support Array map with more than 1 parameters, but it should not add another overloads to map, but rename the function instead for these cases.

  • for the length, it should be type error for TS, and undefined/exception (for JS).

Thank you.