ReadonlyArray<T> instead of T[]

[AbakumovAlexandr]

Feature description:

A proposal is to use ReadonlyArray<T> instead of T[] for input (as seen from NGB side) arrays throughout the publicly visible API, for instance, as a return value type for ngbTypeahead function in NgbTypeahead:

(text: Observable<string>) => Observable<ReadonlyArray<any>>

instead of:

(text: Observable<string>) => Observable<any[]>

It explicitly declares by NGB the immutability of arrays as well as ensures that both groups of developers who use/don't use ReadonlyArray<T> would be able to communicate with NGB without extra effort like array.slice() before passing to\returning from NGB methods.

The above part doesn't introduce a breaking change. We could also consider go further and do the same for output arrays as well to promote immutability in the client code bases.

Versions of Angular, ng-bootstrap and Bootstrap:

ng-bootstrap: 3.3.0

[pkozlowski-opensource]

Sounds like a sensible request. Small, focused (separate one per widget) PRs welcomed.

[benouat]

would be able to communicate with NGB without extra effort like array.slice() before passing to\returning from NGB methods

Could you illustrate this extra effort you're mentioning? I am not sure to get what you are describing here.

[AbakumovAlexandr]

@benouat Sure. Let's take the above example with ngbTypeahead. Its signature requires a function to return any[] instead of ReadonlyArray<any>.

Now, suppose our app conforms to immutability ideology and uses ReadonlyArrays only. To implement the ngbTypeahead function, we thus have to somehow convert our ReadonlyArray<any> to any[] before returning it to NGB.

The first thought is just to perform a type cast:

public typeaheadListFn = (text: Observable<string>): Observable<any[]> => {
        const list: ReadonlyArray<any> = this.getTypeageadListForText(text);
        return list as any[];
    };

But we shouldn't do this, since if NGB actually calls any mutating method on that array which is missing in our ReadonlyArray<any>, there would be a run-time exception.

So, the simplest way I'm aware of is to make a copy of our immutable array with slice() before returning it:

public typeaheadListFn = (text: Observable<string>): Observable<any[]> => {
        const list: ReadonlyArray<any> = this.getTypeageadListForText(text);
        return list.slice();
    };

It would create a mutable copy of our array.

[AbakumovAlexandr]

@pkozlowski-opensource @benouat I haven't checked it out, but I believe it's not specific to typeahead only. I propose to implement this approach across all the components and NGB API.

[peterblazejewicz]

Just update to the syntax, In the TypeScript 3.4 (released this year, after the original problem creation), the readonly modifier was introduced to allow to be less verbose in specifying the intent, so instead of using (pseudo code):

someArray: ReadonlyArray<T>  = [];

one could simply update existing decclaration with:

someArray:  readonly T[] = [];

as the const messages: ReadonlyArray<string> = ["Hello World"]; resolves to const messages: readonly string[]

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#improvements-for-readonlyarray-and-readonly-tuples

[maxokorokov]

Closed in #3423, #3424 and #3426