fix collection index incrementing when performing insertBefore

previously was only checking elements in the collection  after the loading spinner node, thus items loaded async into the collection all remained with incorrect index, resulting in incorrect aria-rowIndex

Author: LFDanLu

packages/@react-aria/collections/src/Document.ts

@@ -155,7 +155,6 @@ export class BaseNode<T> {
     newNode.nextSibling = referenceNode;
     newNode.previousSibling = referenceNode.previousSibling;
     newNode.index = referenceNode.index;
-
     if (this.firstChild === referenceNode) {
       this.firstChild = newNode;
     } else if (referenceNode.previousSibling) {
@@ -165,15 +164,15 @@ export class BaseNode<T> {
     referenceNode.previousSibling = newNode;
     newNode.parentNode = referenceNode.parentNode;
 
-    this.invalidateChildIndices(referenceNode);
+    this.invalidateChildIndices(newNode);
     this.ownerDocument.queueUpdate();
   }
 
   removeChild(child: ElementNode<T>): void {
     if (child.parentNode !== this || !this.ownerDocument.isMounted) {
       return;
     }
-    
+
     if (child.nextSibling) {
       this.invalidateChildIndices(child.nextSibling);
       child.nextSibling.previousSibling = child.previousSibling;
@@ -279,7 +278,7 @@ export class ElementNode<T> extends BaseNode<T> {
       this.node = this.node.clone();
       this.isMutated = true;
     }
-    
+
     this.ownerDocument.markDirty(this);
     return this.node;
   }
@@ -497,7 +496,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
     if (this.dirtyNodes.size === 0 || this.queuedRender) {
       return;
     }
-    
+
     // Only trigger subscriptions once during an update, when the first item changes.
     // React's useSyncExternalStore will call getCollection immediately, to check whether the snapshot changed.
     // If so, React will queue a render to happen after the current commit to our fake DOM finishes.

packages/@react-aria/virtualizer/src/ScrollView.tsx

@@ -199,7 +199,6 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
   // Update visible rect when the content size changes, in case scrollbars need to appear or disappear.
   let lastContentSize = useRef<Size | null>(null);
   let [update, setUpdate] = useState({});
-
   useLayoutEffect(() => {
     if (!isUpdatingSize.current && (lastContentSize.current == null || !contentSize.equals(lastContentSize.current))) {
       // React doesn't allow flushSync inside effects, so queue a microtask.
@@ -210,8 +209,8 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
       // https://github.com/reactwg/react-18/discussions/102
       // @ts-ignore
       if (typeof IS_REACT_ACT_ENVIRONMENT === 'boolean' ? IS_REACT_ACT_ENVIRONMENT : typeof jest !== 'undefined') {
-        // Queue call of updateSize to happen in a separate render but within the same act so that RAC virtualized ComboBoxes and Selects
-        // work properly
+        // This is so we update size in a separate render but within the same act. Needs to be setState instead of refs
+        // due to strict mode.
         setUpdate({});
         lastContentSize.current = contentSize;
         return
@@ -223,6 +222,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
     lastContentSize.current = contentSize;
   });
 
+  // Will only run in tests, needs to be in separate effect so it is properly run in the next render in strict mode.
   useLayoutEffect(() => {
     updateSize(fn => fn());
   }, [update])

packages/react-aria-components/test/Table.test.js

@@ -2031,23 +2031,13 @@ describe('Table', () => {
       beforeAll(() => {
         clientWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100);
         clientHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100);
-        // jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 150);
-        // jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100);
-        // jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementationOnce(() => 0).mockImplementation(function () {
-        //   if (this.getAttribute('role') === 'grid') {
-        //     return 50;
-        //   }
-
-        //   return 25;
-        // });
       });
 
       beforeEach(() => {
         act(() => {jest.runAllTimers();});
       });
 
       afterAll(function () {
-        // TODO fix these
         clientWidth.mockReset();
         clientHeight.mockReset();
       });
@@ -2137,6 +2127,11 @@ describe('Table', () => {
         expect(sentinelParentStyles.top).toBe('0px');
         expect(sentinelParentStyles.height).toBe('30px');
       });
+
+      it('should have the correct row indicies after loading more items', async () => {
+        // TODO: first render without items and is loading
+        // then render with items and double check
+      });
     });
   });