Fix RAC collections in React canary versions (#4518)

Author: devongovett

.circleci/config.yml

@@ -108,6 +108,26 @@ jobs:
             - ~/react-spectrum
           key: react-spectrum17-{{ .Environment.CACHE_VERSION }}-{{ .Environment.CIRCLE_SHA1 }}
 
+  install-canary:
+    executor: rsp-large
+    steps:
+      - checkout
+      - restore_cache:
+          keys:
+            - rsp-yarn-{{ .Environment.CACHE_VERSION }}-{{ .Branch }}-{{ checksum "yarn.lock" }}
+            - rsp-yarn-{{ .Environment.CACHE_VERSION }}-{{ .Branch }}-
+            - rsp-yarn-{{ .Environment.CACHE_VERSION }}-
+
+      - run:
+          name: build
+          command: |
+            yarn install --pure-lockfile --cache-folder ~/.cache/yarn && yarn install-canary --cache-folder ~/.cache/yarn
+
+      - save_cache:
+          paths:
+            - ~/react-spectrum
+          key: react-spectrum-canary-{{ .Environment.CACHE_VERSION }}-{{ .Environment.CIRCLE_SHA1 }}
+
   test-ssr:
     executor: rsp-xlarge
     steps:
@@ -166,6 +186,17 @@ jobs:
           command: |
             yarn test:ssr
 
+  test-ssr-canary:
+    executor: rsp-xlarge
+    steps:
+      - restore_cache:
+          key: react-spectrum-canary-{{ .Environment.CACHE_VERSION }}-{{ .Environment.CIRCLE_SHA1 }}
+
+      - run:
+          name: test ssr
+          command: |
+            yarn test:ssr
+
   test-16:
     parallelism: 3
     executor: rsp-xlarge
@@ -214,6 +245,29 @@ jobs:
       - store_artifacts:
           path: ~/junit
 
+  test-canary:
+    parallelism: 3
+    executor: rsp-xlarge
+    steps:
+      - restore_cache:
+          key: react-spectrum-canary-{{ .Environment.CACHE_VERSION }}-{{ .Environment.CIRCLE_SHA1 }}
+
+      - run: mkdir ~/junit
+
+      - run:
+          name: test
+          command: |
+            shopt -s globstar
+            TESTFILES=$(circleci tests glob "packages/**/*.test.[tj]{s,sx}" | circleci tests split --split-by=timings)
+            JEST_JUNIT_OUTPUT_NAME="junit-canary.xml" yarn test ${TESTFILES}
+
+      - run:
+          command: cp junit-canary.xml ~/junit/
+          when: always
+      - store_test_results:
+          path: ~/junit
+      - store_artifacts:
+          path: ~/junit
 
   test-esm:
     executor: rsp-xlarge-nodeupdate
@@ -465,6 +519,7 @@ workflows:
       - install
       - install-16
       - install-17
+      - install-canary
       - test-ssr:
           requires:
             - install
@@ -483,6 +538,12 @@ workflows:
       - test-17:
           requires:
             - install-17
+      - test-ssr-canary:
+          requires:
+            - install-canary
+      - test-canary:
+          requires:
+            - install-canary
       - test-esm:
           requires:
             - install
@@ -544,6 +605,9 @@ workflows:
             - test-16
             - test-ssr-17
             - test-17
+            - test-ssr-canary
+            - test-canary
+            - test-ssr-canary
             - test-esm
             - storybook
             - storybook-16

package.json

@@ -11,6 +11,7 @@
     "check-types": "tsc && tsc-strict",
     "install-16": "yarn add -W react@^16.8.0 react-dom@^16.8.0 @testing-library/react@^12 @testing-library/react-hooks@^8 @testing-library/dom@^8",
     "install-17": "yarn add -W react@^17 react-dom@^17 @testing-library/react@^12 @testing-library/react-hooks@^8 @testing-library/dom@^8",
+    "install-canary": "yarn add -W react@canary react-dom@canary",
     "start": "cross-env NODE_ENV=storybook start-storybook -p 9003 --ci -c '.storybook'",
     "build:storybook": "build-storybook -c .storybook -o dist/$(git rev-parse HEAD)/storybook",
     "build:storybook-16": "build-storybook -c .storybook -o dist/$(git rev-parse HEAD)/storybook-16",

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

@@ -262,6 +262,7 @@ export class ElementNode<T> extends BaseNode<T> {
   nodeType = 8; // COMMENT_NODE (we'd use ELEMENT_NODE but React DevTools will fail to get its dimensions)
   node: NodeValue<T>;
   private _index: number = 0;
+  private hasSetProps = false;
 
   constructor(type: string, ownerDocument: Document<T, any>) {
     super(ownerDocument);
@@ -297,21 +298,20 @@ export class ElementNode<T> extends BaseNode<T> {
     node.lastChildKey = this.lastChild?.node.key ?? null;
   }
 
-  // Special property that React passes through as an object rather than a string via setAttribute.
-  // See below for details.
-  set multiple(obj: any) {
+  setProps(obj: any, rendered?: ReactNode) {
     let node = this.ownerDocument.getMutableNode(this);
-    let {rendered, value, textValue, id, ...props} = obj;
+    let {value, textValue, id, ...props} = obj;
     node.props = props;
     node.rendered = rendered;
     node.value = value;
     node.textValue = textValue || (typeof rendered === 'string' ? rendered : '') || obj['aria-label'] || '';
     if (id != null && id !== node.key) {
-      if (this.parentNode) {
+      if (this.hasSetProps) {
         throw new Error('Cannot change the id of an item');
       }
       node.key = id;
     }
+    this.hasSetProps = true;
   }
 
   get style() {
@@ -672,11 +672,13 @@ export function useCollection<T extends object, C extends BaseCollection<T>>(pro
 }
 
 /** Renders a DOM element (e.g. separator or header) shallowly when inside a collection. */
-export function useShallowRender<T extends Element>(Element: string, props: React.HTMLAttributes<T>, ref: React.RefObject<T>): ReactElement | null {
+export function useShallowRender<T extends Element>(Element: string, props: React.HTMLAttributes<T>, ref: React.Ref<T>): ReactElement | null {
   let isShallow = useContext(ShallowRenderContext);
+  props = useMemo(() => ({...props, ref}), [props, ref]);
+  ref = useCollectionItemRef(props, props.children);
   if (isShallow) {
     // @ts-ignore
-    return <Element multiple={{...props, ref, rendered: props.children}} />;
+    return <Element ref={ref} />;
   }
 
   return null;
@@ -738,6 +740,13 @@ export interface ItemRenderProps {
   isDropTarget?: boolean
 }
 
+export function useCollectionItemRef(props: any, rendered?: ReactNode) {
+  // Return a callback ref that sets the props object on the fake DOM node.
+  return useCallback((element) => {
+    element?.setProps(props, rendered);
+  }, [props, rendered]);
+}
+
 export interface ItemProps<T = object> extends Omit<SharedItemProps<T>, 'children'>, RenderProps<ItemRenderProps> {
   /** The unique id of the item. */
   id?: Key,
@@ -746,13 +755,8 @@ export interface ItemProps<T = object> extends Omit<SharedItemProps<T>, 'childre
 }
 
 export function Item<T extends object>(props: ItemProps<T>): JSX.Element {
-  // HACK: the `multiple` prop is special in that React will pass it through as a property rather
-  // than converting to a string and using setAttribute. This allows our custom fake DOM to receive
-  // the props as an object. Once React supports custom elements, we can switch to that instead.
-  // https://github.com/facebook/react/issues/11347
-  // https://github.com/facebook/react/blob/82c64e1a49239158c0daa7f0d603d2ad2ee667a9/packages/react-dom/src/shared/DOMProperty.js#L386
   // @ts-ignore
-  return <item multiple={{...props, rendered: props.children}} />;
+  return <item ref={useCollectionItemRef(props, props.children)} />;
 }
 
 export interface SectionProps<T> extends Omit<SharedSectionProps<T>, 'children' | 'title'>, DOMProps {
@@ -768,7 +772,7 @@ export function Section<T extends object>(props: SectionProps<T>): JSX.Element {
   let children = useCollectionChildren(props);
 
   // @ts-ignore
-  return <section multiple={{...props, rendered: props.title}}>{children}</section>;
+  return <section ref={useCollectionItemRef(props)}>{children}</section>;
 }
 
 export const CollectionContext = createContext<CachedChildrenOptions<unknown> | null>(null);

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

@@ -1,5 +1,5 @@
 import {AriaLabelingProps} from '@react-types/shared';
-import {BaseCollection, CollectionContext, CollectionProps, CollectionRendererContext, ItemRenderProps, NodeValue, useCachedChildren, useCollection, useCollectionChildren} from './Collection';
+import {BaseCollection, CollectionContext, CollectionProps, CollectionRendererContext, ItemRenderProps, NodeValue, useCachedChildren, useCollection, useCollectionChildren, useCollectionItemRef} from './Collection';
 import {buildHeaderRows} from '@react-stately/table';
 import {ButtonContext} from './Button';
 import {CheckboxContext} from './Checkbox';
@@ -388,7 +388,7 @@ export function TableHeader<T extends object>(props: TableHeaderProps<T>) {
   return (
     <CollectionRendererContext.Provider value={renderer}>
       {/* @ts-ignore */}
-      <tableheader multiple={props}>{children}</tableheader>
+      <tableheader ref={useCollectionItemRef(props)}>{children}</tableheader>
     </CollectionRendererContext.Provider>
   );
 }
@@ -442,7 +442,7 @@ export function Column<T extends object>(props: ColumnProps<T>): JSX.Element {
   });
 
   // @ts-ignore
-  return <column multiple={{...props, rendered: props.title ?? props.children}}>{children}</column>;
+  return <column ref={useCollectionItemRef(props, props.title ?? props.children)}>{children}</column>;
 }
 
 export interface TableBodyRenderProps {
@@ -465,7 +465,7 @@ export function TableBody<T extends object>(props: TableBodyProps<T>) {
   let children = useCollectionChildren(props);
 
   // @ts-ignore
-  return <tablebody multiple={props}>{children}</tablebody>;
+  return <tablebody ref={useCollectionItemRef(props)}>{children}</tablebody>;
 }
 
 export interface RowRenderProps extends ItemRenderProps {}
@@ -494,7 +494,7 @@ export function Row<T extends object>(props: RowProps<T>) {
 
   return (
     // @ts-ignore
-    <item multiple={props}>
+    <item ref={useCollectionItemRef(props)}>
       <CollectionContext.Provider value={ctx}>
         {children}
       </CollectionContext.Provider>
@@ -534,7 +534,7 @@ export interface CellProps extends RenderProps<CellRenderProps> {
  */
 export function Cell(props: CellProps): JSX.Element {
   // @ts-ignore
-  return <cell multiple={{...props, rendered: props.children}} />;
+  return <cell ref={useCollectionItemRef(props, props.children)} />;
 }
 
 function TableHeaderRowGroup<T>({collection}: {collection: TableCollection<T>}) {