Skip to content

Conversation

@devongovett
Copy link
Member

@devongovett devongovett commented May 27, 2023

Closes #4470

The original issue was that Framer Motion's types appeared not to be compatible with some React Aria Components, e.g. motion(Modal). After playing around for a while, I realized that this had to do with forwardRef.

Then, I found that I could not re-create the issue within our repo. It turned out that we are on an older version of @types/react, and the type of react children (ReactNode) changed in newer versions breaking this use case. I updated this, and it revealed a lot of errors which I have fixed in the first commit to this PR.

With those fixed, I was finally able to recreate the original issue and added a file with a test for each component to ensure it was compatible. The ones that were broken were the ones that support functions as children, e.g. for render props. Not sure exactly why forwardRef breaks this, but I was able to fix the issue the same way as we did for generics (using an overridden function type).

@rspbot
Copy link

rspbot commented May 27, 2023

@rspbot
Copy link

rspbot commented May 31, 2023

snowystinger
snowystinger previously approved these changes May 31, 2023
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is what you meant when you said the generic were working finally? ReactElement<TableHeaderProps<T>> we've got some other places we could do this now :)

LFDanLu
LFDanLu previously approved these changes Jun 2, 2023
ktabors
ktabors previously approved these changes Jun 2, 2023
…act-types

# Conflicts:
#	packages/@react-aria/virtualizer/src/Virtualizer.tsx
#	packages/@react-aria/virtualizer/src/VirtualizerItem.tsx
#	packages/@react-spectrum/table/src/TableView.tsx
@devongovett devongovett dismissed stale reviews from ktabors, LFDanLu, and snowystinger via b9185de June 2, 2023 22:17
@rspbot
Copy link

rspbot commented Jun 2, 2023

@rspbot
Copy link

rspbot commented Jun 2, 2023

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
undefined already in set

@react-aria/button

useButton

 useButton {
-  props: AriaButtonProps<ElementType>
+  props: AriaButtonOptions<ElementType>
   ref: RefObject<any>
   returnVal: undefined
 }

useToggleButton

 useToggleButton {
-  props: AriaToggleButtonProps<ElementType>
+  props: AriaToggleButtonOptions<ElementType>
   state: ToggleState
   ref: RefObject<any>
   returnVal: undefined
 }

@react-aria/virtualizer

useVirtualizer

-useVirtualizer<T extends {}, V, W> {
-  props: VirtualizerOptions
-  state: VirtualizerState<T, V, W>
-  ref: RefObject<HTMLElement>
-  returnVal: undefined
-}
+

Virtualizer

-Virtualizer<T extends {}, V> {
-  autoFocus?: boolean
-  children: (string, {}) => V
-  collection: Collection<{}>
-  focusedKey?: Key
-  isLoading?: boolean
-  layout: Layout<{}>
-  onLoadMore?: () => void
-  renderWrapper?: (ReusableView<{}, V> | null, ReusableView<{}, V>, Array<ReusableView<{}, V>>, (Array<ReusableView<{}, V>>) => Array<ReactElement>) => ReactElement
-  scrollDirection?: 'horizontal' | 'vertical' | 'both'
-  scrollToItem?: (Key) => void
-  shouldUseVirtualFocus?: boolean
-  sizeToFit?: 'width' | 'height'
-  transitionDuration?: number
-}
+

undefined

-
+useVirtualizer<T extends {}, V extends ReactNode, W> {
+  props: VirtualizerOptions
+  state: VirtualizerState<T, V, W>
+  ref: RefObject<HTMLElement>
+  returnVal: undefined
+}

undefined

-
+Virtualizer<T extends {}, V extends ReactNode> {
+  autoFocus?: boolean
+  children: (string, {}) => ReactNode
+  collection: Collection<{}>
+  focusedKey?: Key
+  isLoading?: boolean
+  layout: Layout<{}>
+  onLoadMore?: () => void
+  renderWrapper?: (ReusableView<{}, ReactNode> | null, ReusableView<{}, ReactNode>, Array<ReusableView<{}, ReactNode>>, (Array<ReusableView<{}, ReactNode>>) => Array<ReactElement>) => ReactElement
+  scrollDirection?: 'horizontal' | 'vertical' | 'both'
+  scrollToItem?: (Key) => void
+  shouldUseVirtualFocus?: boolean
+  sizeToFit?: 'width' | 'height'
+  transitionDuration?: number
+}

@react-spectrum/actionbar

Item

 SpectrumActionBarProps<T> {
+  children: ItemElement<T> | Array<ItemElement<T>> | ItemRenderer<T>
+  disabledKeys?: Iterable<Key>
   isEmphasized?: boolean
+  items?: Iterable<T>
   onAction?: (Key) => void
   onClearSelection: () => void
   selectedItemCount: number | 'all'
 }

@react-spectrum/table

TableHeaderProps

 TableBodyProps<T> {
-  children: CollectionChildren<T>
+  children: RowElement | Array<RowElement> | (T) => RowElement
   items?: Iterable<T>
   loadingState?: LoadingState
 }

@react-stately/collections

PartialNode

changed by:

  • PartialNode
 PartialNode<T> {
   aria-label?: string
   childNodes?: () => IterableIterator<PartialNode<T>>
   element?: ReactElement
   hasChildNodes?: boolean
   index?: number
   key?: Key
   props?: any
   rendered?: ReactNode
-  renderer?: ItemRenderer<T>
+  renderer?: (T) => ReactElement
   shouldInvalidate?: (unknown) => boolean
   textValue?: string
   type?: string
   value?: T
 }
 

it changed:

  • PartialNode

useCollection

 useCollection<C extends Collection<Node<T>> = Collection<Node<T>>, T extends {}> {
-  props: CollectionStateBase<T, C>
+  props: CollectionOptions<T, C>
   factory: CollectionFactory<T, C>
   context?: unknown
   returnVal: undefined
 }

@react-stately/table

TableStateProps

 TableStateProps<T> {
+  children?: [ReactElement<TableHeaderProps<T>>, ReactElement<TableBodyProps<T>>]
+  collection?: ITableCollection<T>
+  disabledKeys?: Iterable<Key>
   showSelectionCheckboxes?: boolean
 }

it changed:

  • useTableState

TableBodyProps

 TableBodyProps<T> {
-  children: CollectionChildren<T>
+  children: RowElement | Array<RowElement> | (T) => RowElement
   items?: Iterable<T>
   loadingState?: LoadingState
 }

@react-stately/toggle

useToggleState

changed by:

  • ToggleStateOptions
 useToggleState {
-  props: ToggleProps
+  props: ToggleStateOptions
   returnVal: undefined
 }

ToggleStateOptions

+ToggleStateOptions {
 
+}

it changed:

  • useToggleState

@devongovett devongovett merged commit 1123b0f into main Jun 2, 2023
@devongovett devongovett deleted the update-react-types branch June 2, 2023 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MyComboBoxProps has incorrect children type in react-aria-component ComboBox docs

6 participants