Improve types for some array internals

Author: wagenet

packages/@ember/-internals/metal/lib/array.ts

@@ -1,10 +1,13 @@
-import { Array as EmberArray, MutableArray, NativeArray } from '@ember/-internals/runtime';
+import { Array as EmberArray, MutableArray } from '@ember/-internals/runtime';
+import { assert } from '@ember/debug';
 import { arrayContentDidChange, arrayContentWillChange } from './array_events';
 import { addListener, removeListener } from './events';
 
 const EMPTY_ARRAY = Object.freeze([]);
 
-interface ObservedObject<T> extends EmberArray<T> {
+type ObservedArray<T> = (T[] | EmberArray<T>) & ObservedObject;
+
+interface ObservedObject {
   _revalidate?: () => void;
 }
 
@@ -16,16 +19,23 @@ export function objectAt<T>(array: T[] | EmberArray<T>, index: number): T | unde
   }
 }
 
+// Ideally, we'd use MutableArray.detect but for unknown reasons this causes
+// the node tests to fail strangely.
+function isMutableArray<T>(obj: unknown): obj is MutableArray<T> {
+  return obj != null && typeof (obj as MutableArray<T>).replace === 'function';
+}
+
 export function replace<T>(
-  array: NativeArray<T> | MutableArray<T>,
+  array: T[] | MutableArray<T>,
   start: number,
   deleteCount: number,
   items: T[] = EMPTY_ARRAY as []
 ): void {
-  if (Array.isArray(array)) {
-    replaceInNativeArray(array, start, deleteCount, items);
-  } else {
+  if (isMutableArray(array)) {
     array.replace(start, deleteCount, items);
+  } else {
+    assert('Can only replace content of a native array or MutableArray', Array.isArray(array));
+    replaceInNativeArray(array, start, deleteCount, items);
   }
 }
 
@@ -34,7 +44,7 @@ const CHUNK_SIZE = 60000;
 // To avoid overflowing the stack, we splice up to CHUNK_SIZE items at a time.
 // See https://code.google.com/p/chromium/issues/detail?id=56588 for more details.
 export function replaceInNativeArray<T>(
-  array: T[] | NativeArray<T>,
+  array: T[],
   start: number,
   deleteCount: number,
   items: ReadonlyArray<T>
@@ -61,18 +71,18 @@ interface ArrayObserverOptions {
 }
 
 type Operation<T> = (
-  obj: ObservedObject<T>,
+  obj: ObservedArray<T>,
   eventName: string,
   target: object | Function | null,
   callbackName: string
 ) => void;
 
 function arrayObserversHelper<T>(
-  obj: ObservedObject<T>,
+  obj: ObservedArray<T>,
   target: object | Function | null,
   opts: ArrayObserverOptions,
   operation: Operation<T>
-): ObservedObject<T> {
+): ObservedArray<T> {
   let { willChange, didChange } = opts;
 
   operation(obj, '@array:before', target, willChange);
@@ -91,14 +101,14 @@ export function addArrayObserver<T>(
   array: EmberArray<T>,
   target: object | Function | null,
   opts: ArrayObserverOptions
-): ObservedObject<T> {
+): ObservedArray<T> {
   return arrayObserversHelper(array, target, opts, addListener);
 }
 
 export function removeArrayObserver<T>(
-  array: EmberArray<T>,
+  array: T[] | EmberArray<T>,
   target: object | Function | null,
   opts: ArrayObserverOptions
-): ObservedObject<T> {
+): ObservedArray<T> {
   return arrayObserversHelper(array, target, opts, removeListener);
 }

packages/@ember/-internals/runtime/lib/mixins/array.d.ts

@@ -107,4 +107,4 @@ export function uniqBy<T>(
   array: T[] | EmberArray<T>,
   keyOrFunc: string | ((item: T) => unknown)
 ): T[] | EmberArray<T>;
-export function isArray(obj: unknown): boolean;
+export function isArray(obj: unknown): obj is ArrayLike<unknown> | EmberArray<unknown>;

packages/@ember/-internals/runtime/lib/system/array_proxy.ts

@@ -16,7 +16,7 @@ import {
 } from '@ember/-internals/metal';
 import { isObject } from '@ember/-internals/utils';
 import EmberObject from './object';
-import EmberArray, { isArray, MutableArray } from '../mixins/array';
+import EmberArray, { MutableArray } from '../mixins/array';
 import { assert } from '@ember/debug';
 import { setCustomTagFor } from '@glimmer/manager';
 import {
@@ -30,7 +30,7 @@ import {
 } from '@glimmer/validator';
 import { PropertyDidChange } from '@ember/-internals/metal/lib/property_events';
 
-function isMutable<T>(obj: EmberArray<T>): obj is MutableArray<T> {
+function isMutable<T>(obj: T[] | EmberArray<T>): obj is T[] | MutableArray<T> {
   return Array.isArray(obj) || typeof (obj as MutableArray<T>).replace === 'function';
 }
 
@@ -116,8 +116,8 @@ function customTagForArrayProxy(proxy: object, key: string) {
   @public
 */
 // eslint-disable-next-line @typescript-eslint/no-empty-interface, @typescript-eslint/no-unused-vars
-interface ArrayProxy<T, C extends EmberArray<T> = EmberArray<T>> extends MutableArray<T> {}
-class ArrayProxy<T, C extends EmberArray<T> = EmberArray<T>>
+interface ArrayProxy<T, C extends EmberArray<T> | T[] = T[]> extends MutableArray<T> {}
+class ArrayProxy<T, C extends EmberArray<T> | T[] = T[]>
   extends EmberObject
   implements PropertyDidChange
 {
@@ -218,15 +218,15 @@ class ArrayProxy<T, C extends EmberArray<T> = EmberArray<T>>
     @method replaceContent
     @param {Number} idx The starting index
     @param {Number} amt The number of items to remove from the content.
-    @param {EmberArray} objects Optional array of objects to insert.
+    @param {Array} objects Optional array of objects to insert.
     @return {void}
     @public
   */
   replaceContent(idx: number, amt: number, objects?: T[]) {
     let content = get(this, 'content');
-    assert('[BUG] Called objectAtContent without content', content);
+    assert('[BUG] Called replaceContent without content', content);
     assert('Mutating a non-mutable array is not allowed', isMutable(content));
-    content.replace(idx, amt, objects);
+    replace<T>(content, idx, amt, objects);
   }
 
   // Overriding objectAt is not supported.
@@ -312,8 +312,14 @@ class ArrayProxy<T, C extends EmberArray<T> = EmberArray<T>>
       // @ts-expect-error This check is still good for ensuring correctness
       assert("Can't set ArrayProxy's content to itself", arrangedContent !== this);
       assert(
-        `ArrayProxy expects an Array or ArrayProxy, but you passed ${typeof arrangedContent}`,
-        isArray(arrangedContent) || (arrangedContent as any).isDestroyed
+        `ArrayProxy expects a native Array, EmberArray, or ArrayProxy, but you passed ${typeof arrangedContent}`,
+        (function (arr: unknown): arr is EmberArray<unknown> {
+          return Array.isArray(arr) || EmberArray.detect(arr);
+        })(arrangedContent)
+      );
+      assert(
+        'ArrayProxy expected its contents to not be destroyed',
+        !(arrangedContent as any).isDestroyed
       );
 
       addArrayObserver(arrangedContent, this, ARRAY_OBSERVER_MAPPING);