Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RAC collections in React canary versions #4518

Merged
merged 2 commits into from
May 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -465,6 +519,7 @@ workflows:
- install
- install-16
- install-17
- install-canary
- test-ssr:
requires:
- install
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
32 changes: 18 additions & 14 deletions packages/react-aria-components/src/Collection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions packages/react-aria-components/src/Table.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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>
);
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {}
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>}) {
Expand Down