fix tableCollection filter so it doesnt need to call filterChildren directly

Author: LFDanLu

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

@@ -233,16 +233,19 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
     this.frozen = !isSSR;
   }
 
-  filter(filterFn: (textValue: string) => boolean): BaseCollection<T> {
-    let newCollection = new BaseCollection<T>();
+  filter(filterFn: (textValue: string) => boolean, newCollection?: BaseCollection<T>): BaseCollection<T> {
+    if (newCollection == null) {
+      newCollection = new BaseCollection<T>();
+    }
+
     let [firstKey, lastKey] = filterChildren(this, newCollection, this.firstKey, filterFn);
     newCollection.firstKey = firstKey;
     newCollection.lastKey = lastKey;
     return newCollection;
   }
 }
 
-export function filterChildren<T>(collection: BaseCollection<T>, newCollection: BaseCollection<T>, firstChildKey: Key | null, filterFn: (textValue: string) => boolean): [Key | null, Key | null] {
+function filterChildren<T>(collection: BaseCollection<T>, newCollection: BaseCollection<T>, firstChildKey: Key | null, filterFn: (textValue: string) => boolean): [Key | null, Key | null] {
   // loop over the siblings for firstChildKey
   // create new nodes based on calling node.filter for each child
   // if it returns null then don't include it, otherwise update its prev/next keys

packages/@react-aria/collections/src/CollectionBuilder.tsx

@@ -140,15 +140,13 @@ function useSSRCollectionNode<T extends Element>(CollectionNodeClass: Collection
   // collection by the time we need to use the collection to render to the real DOM.
   // After hydration, we switch to client rendering using the portal.
   let itemRef = useCallback((element: ElementNode<any> | null) => {
-    // TODO: check setProps api
     element?.setProps(props, ref, rendered, render, CollectionNodeClass);
   }, [props, ref, rendered, render, CollectionNodeClass]);
   let parentNode = useContext(SSRContext);
   if (parentNode) {
     // Guard against double rendering in strict mode.
     let element = parentNode.ownerDocument.nodesByProps.get(props);
     if (!element) {
-      // TODO: check this, maybe should just pass the CollectionNodeClass as a whole?
       element = parentNode.ownerDocument.createElement(CollectionNodeClass.type);
       element.setProps(props, ref, rendered, render);
       parentNode.appendChild(element);
@@ -161,7 +159,6 @@ function useSSRCollectionNode<T extends Element>(CollectionNodeClass: Collection
       : null;
   }
 
-  // console.log('type', CollectionNodeClass, CollectionNodeClass.type)
   // @ts-ignore
   // TODO: could just make this a div perhaps, but keep it in line with how it used to work
   return <CollectionNodeClass.type ref={itemRef}>{children}</CollectionNodeClass.type>;
@@ -201,7 +198,6 @@ export function createLeafComponent<P extends object, E extends Element>(Collect
   return Result;
 }
 
-// TODO: check the signature of this too
 export function createBranchComponent<T extends object, P extends {children?: any}, E extends Element>(CollectionNodeClass: CollectionNodeClass<any>, render: (props: P, ref: ForwardedRef<E>, node: Node<T>) => ReactElement | null, useChildren: (props: P) => ReactNode = useCollectionChildren): (props: P & React.RefAttributes<E>) => ReactElement | null {
   let Component = ({node}) => render(node.props, node.props.ref, node);
   let Result = (forwardRef as forwardRefType)((props: P, ref: ForwardedRef<E>) => {

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

@@ -324,7 +324,6 @@ export class ElementNode<T> extends BaseNode<T> {
     }
   }
 
-  // TODO
   setProps<E extends Element>(obj: {[key: string]: any}, ref: ForwardedRef<E>, rendered?: ReactNode, render?: (node: Node<T>) => ReactElement, CollectionNodeClass?: CollectionNodeClass<any>): void {
     let node = this.getMutableNode();
     let {value, textValue, id, ...props} = obj;

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

@@ -13,7 +13,7 @@
 export {CollectionBuilder, Collection, createLeafComponent, createBranchComponent} from './CollectionBuilder';
 export {createHideableComponent, useIsHidden} from './Hidden';
 export {useCachedChildren} from './useCachedChildren';
-export {BaseCollection, CollectionNode, filterChildren} from './BaseCollection';
+export {BaseCollection, CollectionNode} from './BaseCollection';
 
 export type {CollectionBuilderProps, CollectionProps} from './CollectionBuilder';
 export type {CachedChildrenOptions} from './useCachedChildren';

packages/@react-stately/table/src/useTableState.ts

@@ -109,7 +109,7 @@ export function useTableState<T extends object>(props: TableStateProps<T>): Tabl
 }
 
 /**
- * Filters a collection using the provided filter function and returns a new ListState.
+ * Filters a collection using the provided filter function and returns a new TableState.
  */
 export function UNSTABLE_useFilteredTableState<T extends object>(state: TableState<T>, filterFn: ((nodeValue: string) => boolean) | null | undefined): TableState<T> {
   let collection = useMemo(() => filterFn ? state.collection.filter!(filterFn) : state.collection, [state.collection, filterFn]) as ITableCollection<T>;

packages/react-aria-components/src/Breadcrumbs.tsx

@@ -73,13 +73,17 @@ export interface BreadcrumbProps extends RenderProps<BreadcrumbRenderProps>, Glo
   id?: Key
 }
 
-// TODO: perhaps this should be reuse ItemNode, for now just have it separate
 class BreadcrumbNode extends CollectionNode<any> {
   static readonly type = 'item';
 
   constructor(key: Key) {
     super(BreadcrumbNode.type, key);
   }
+
+  // For, don't support Breadcrumb filtering
+  filter(): CollectionNode<any> | null {
+    return this.clone();
+  }
 }
 
 /**

packages/react-aria-components/src/GridList.tsx

@@ -284,7 +284,6 @@ export interface GridListItemProps<T = object> extends RenderProps<GridListItemR
   onAction?: () => void
 }
 
-// TODO: reuse?
 class GridListNode extends CollectionNode<any> {
   static readonly type = 'item';
   constructor(key: Key) {
@@ -534,8 +533,6 @@ export interface GridListLoadMoreItemProps extends Omit<LoadMoreSentinelProps, '
   isLoading?: boolean
 }
 
-// TODO: can probably reuse ListBox's loaderNode, but might keep separate
-// add filter logic
 class GridListLoaderNode extends CollectionNode<any> {
   static readonly type = 'loader';
 

packages/react-aria-components/src/ListBox.tsx

@@ -305,8 +305,6 @@ function ListBoxSectionInner<T extends object>(props: ListBoxSectionProps<T>, re
   );
 }
 
-
-// TODO: reuse
 export class ListBoxSectionNode<T> extends CollectionNode<T> {
   static readonly type = 'section';
 
@@ -354,7 +352,6 @@ export interface ListBoxItemProps<T = object> extends RenderProps<ListBoxItemRen
   onAction?: () => void
 }
 
-// TODO: reusue
 class ListBoxItemNode<T> extends CollectionNode<T> {
   static readonly type = 'item';
 
@@ -511,7 +508,6 @@ function ListBoxDropIndicator(props: ListBoxDropIndicatorProps, ref: ForwardedRe
   );
 }
 
-// TODO: can reuse this most likely
 class ListBoxLoaderNode extends CollectionNode<any> {
   static readonly type = 'loader';
 

packages/react-aria-components/src/Table.tsx

@@ -1,5 +1,5 @@
 import {AriaLabelingProps, GlobalDOMAttributes, HoverEvents, Key, LinkDOMProps, PressEvents, RefObject} from '@react-types/shared';
-import {BaseCollection, Collection, CollectionBuilder, CollectionNode, createBranchComponent, createLeafComponent, filterChildren, useCachedChildren} from '@react-aria/collections';
+import {BaseCollection, Collection, CollectionBuilder, CollectionNode, createBranchComponent, createLeafComponent, useCachedChildren} from '@react-aria/collections';
 import {buildHeaderRows, TableColumnResizeState} from '@react-stately/table';
 import {ButtonContext} from './Button';
 import {CheckboxContext} from './RSPContexts';
@@ -165,7 +165,7 @@ class TableCollection<T> extends BaseCollection<T> implements ITableCollection<T
     collection.rowHeaderColumnKeys = this.rowHeaderColumnKeys;
     collection.head = this.head;
     collection.body = this.body;
-    // TODO clone updateColumns as well but it is private
+    collection.updateColumns = this.updateColumns;
     return collection;
   }
 
@@ -198,17 +198,9 @@ class TableCollection<T> extends BaseCollection<T> implements ITableCollection<T
   }
 
   filter(filterFn: (textValue: string) => boolean): TableCollection<T> {
-    // TODO: ideally we wouldn't need to reimplement this but we need a TableCollection, not a BaseCollection
-    // Also need to handle the fact that a bunch of properites are private
-    let clone = this.clone() as Mutable<TableCollection<T>>;
-    // @ts-ignore
-    let [firstKey, lastKey] = filterChildren(this, clone, this.firstKey, filterFn);
-    // @ts-ignore
-    clone.firstKey = firstKey;
-    // @ts-ignore
-    clone.lastKey = lastKey;
-    // @ts-ignore
-    return clone;
+    let clone = this.clone();
+    return super.filter(filterFn, clone) as TableCollection<T>;
+
   }
 }