[react-aria-components] - Dynamic Table with sorting column name in sortDescriptor is random string

[mauron85]

Provide a general summary of the issue here

I'm trying to mix two examples into one working - dynamic table with sorting.

https://react-spectrum.adobe.com/react-aria/Table.html#content
https://react-spectrum.adobe.com/react-aria/Table.html#sorting

The issue is that onSortChange callback doesn't receive sortDescriptor.column as key of the column, but random string react-aria-1...

Also the confusing part is that for dynamic table header columns are defined:

  let columns = [
    {name: 'Name', key: 'name', isRowHeader: true},
    {name: 'Type', key: 'type'},
    {name: 'Date Modified', key: 'date'}
  ];

 <Table
      aria-label="Example table with client side sorting"
      sortDescriptor={list.sortDescriptor}
      onSortChange={list.sort}
    >
      <TableHeader>
        <MyColumn id="name" isRowHeader allowsSorting>Name</MyColumn>
        <MyColumn id="height" allowsSorting>Height</MyColumn>
        <MyColumn id="mass" allowsSorting>Mass</MyColumn>
        <MyColumn id="birth_year" allowsSorting>Birth Year</MyColumn>
      </TableHeader>
      <TableBody items={list.items}>
        {(item) => (
          <Row id={item.name}>
            <Cell>{item.name}</Cell>
            <Cell>{item.height}</Cell>
            <Cell>{item.mass}</Cell>
            <Cell>{item.birth_year}</Cell>
          </Row>
        )}
      </TableBody>
    </Table>

🤔 Expected Behavior?

When using dynamic table with sorting, sortDescriptor.column should be key from columns array definition

😯 Current Behavior

When using dynamic table with sorting, sortDescriptor.column is react-aria-1, react-aria-2...

💁 Possible Solution

No response

🔦 Context

I think the problem is related how Collection is handling items (columns)

🖥️ Steps to Reproduce

https://codesandbox.io/s/wizardly-fast-zsmnww

Version

1.0.0-beta.1

What browsers are you seeing the problem on?

Microsoft Edge

If other, please specify.

No response

What operating system are you using?

Windows 10

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[reidbarber]

Looks like it's using the key created for the fake DOM element:

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

Line 278 in d334cde

this.node = new NodeValue(type, `react-aria-${++ownerDocument.nodeId}`);

Maybe we can pass in the user-provided key for the Column into useSSRCollectionNode to get used instead:

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

Lines 550 to 564 in d334cde

	function Column<T extends object>(props: ColumnProps<T>, ref: ForwardedRef<HTMLTableCellElement>): JSX.Element | null {
	let render = useContext(CollectionRendererContext);
	let childColumns: ReactNode | ((item: T) => ReactNode);
	if (typeof render === 'function') {
	childColumns = render;
	} else if (typeof props.children !== 'function') {
	childColumns = props.children;
	}
	let children = useCollectionChildren({
	children: (props.title || props.childColumns) ? childColumns : null,
	items: props.childColumns
	});
	return useSSRCollectionNode('column', props, ref, props.title ?? props.children, children);

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

Line 858 in d334cde

export function useSSRCollectionNode<T extends Element>(Type: string, props: object, ref: ForwardedRef<T>, rendered?: any, children?: ReactNode) {

[devongovett]

It works if you pass an id prop to the column: https://codesandbox.io/s/condescending-cookies-9d6wsx?file=/src/App.js

<MyColum {...column} id={column.key}>

But I think this is a bug here:

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

Line 663 in d334cde

if (rendered.key == null) {

Since the column object has a key property, and it gets spread on MyColumn, the code in the if statement doesn't run to automatically add id as well. I think this if statement just needs to be deleted potentially.

[mauron85]

Confirming that passing column.key as id to MyColumn fixes the problem.

So my understanding is that key gets swallowed, since key is special prop in React (consumed by the component or by some react internal) and not passed as prop.

EDIT: So if may give suggestion... it would be better for react-components-aria to not use key for column definitions, but rather id would be more appropriate.

https://www.scaler.com/topics/react/list-and-keys-in-react/

Keys Don’t Automatically Get Passed as a Prop to the Component

React doesn't pass the key to your component but only serves as a hint. In simple it means, React does not pass the key ?as a prop automatically to a component. If a unique value is required, we need to pass keys as a prop with a distinct prop >name. Let us take an example to understand better: