-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feat: Add support for multiple adjacent collections #8553
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
Draft
nwidynski
wants to merge
6
commits into
adobe:main
Choose a base branch
from
nwidynski:collection-ref
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
58cba2d
Feat: Add `useCollectionRef` hook to improve controlled collection cases
nwidynski 99eeadb
Fix: mergeRefs immutable tests in React 16 & 17
nwidynski be30beb
chore: update comments
nwidynski 07f7cd3
chore: add synchronization tests
nwidynski 108a757
fix: react 16 render return
nwidynski 39e9a2d
fix: react 16 render return for real this time
nwidynski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,20 +15,38 @@ import {BaseNode, Document, ElementNode} from './Document'; | |
import {CachedChildrenOptions, useCachedChildren} from './useCachedChildren'; | ||
import {createPortal} from 'react-dom'; | ||
import {FocusableContext} from '@react-aria/interactions'; | ||
import {forwardRefType, Node} from '@react-types/shared'; | ||
import {forwardRefType, Node, RefObject} from '@react-types/shared'; | ||
import {Hidden} from './Hidden'; | ||
import React, {createContext, ForwardedRef, forwardRef, JSX, ReactElement, ReactNode, useCallback, useContext, useMemo, useRef, useState} from 'react'; | ||
import React, {createContext, ForwardedRef, forwardRef, JSX, ReactElement, ReactNode, Ref, useCallback, useContext, useMemo, useRef, useState} from 'react'; | ||
import {useIsSSR} from '@react-aria/ssr'; | ||
import {useLayoutEffect} from '@react-aria/utils'; | ||
import {useLayoutEffect, useObjectRef} from '@react-aria/utils'; | ||
import {useSyncExternalStore as useSyncExternalStoreShim} from 'use-sync-external-store/shim/index.js'; | ||
|
||
const ShallowRenderContext = createContext(false); | ||
const CollectionDocumentContext = createContext<Document<any, BaseCollection<any>> | null>(null); | ||
|
||
export interface CollectionProps<T> extends CachedChildrenOptions<T> {} | ||
|
||
export interface CollectionChildren<C extends BaseCollection<object>> { | ||
(collection: C): ReactNode | ||
} | ||
|
||
export interface CollectionRenderProps<C extends BaseCollection<object>> { | ||
/** A hook that will be called before the collection builder to build the content. */ | ||
useCollectionContent?: (content: ReactNode) => typeof content, | ||
/** A hook that will be called by the collection builder to render the children. */ | ||
useCollectionChildren?: (children: CollectionChildren<C>) => typeof children, | ||
/** A hook that will be called by the collection builder to retrieve the collection and document. */ | ||
useCollectionDocument?: (state: CollectionDocumentResult<any, BaseCollection<any>>) => typeof state | ||
} | ||
|
||
interface CollectionRef<C extends BaseCollection<object>, E extends Element> extends RefObject<E | null>, CollectionRenderProps<C> {} | ||
|
||
export interface CollectionBuilderProps<C extends BaseCollection<object>> { | ||
content: ReactNode, | ||
children: (collection: C) => ReactNode, | ||
createCollection?: () => C | ||
children: CollectionChildren<C>, | ||
createCollection?: () => C, | ||
collectionRef?: CollectionRef<C, Element> | ||
} | ||
|
||
/** | ||
|
@@ -37,29 +55,33 @@ export interface CollectionBuilderProps<C extends BaseCollection<object>> { | |
export function CollectionBuilder<C extends BaseCollection<object>>(props: CollectionBuilderProps<C>): ReactElement { | ||
// If a document was provided above us, we're already in a hidden tree. Just render the content. | ||
let doc = useContext(CollectionDocumentContext); | ||
let ref = props.collectionRef ?? {} as CollectionRef<C, Element>; | ||
let content = ref.useCollectionContent ? ref.useCollectionContent(props.content) : props.content; | ||
if (doc) { | ||
// The React types prior to 18 did not allow returning ReactNode from components | ||
// even though the actual implementation since React 16 did. | ||
// We must return ReactElement so that TS does not complain that <CollectionBuilder> | ||
// is not a valid JSX element with React 16 and 17 types. | ||
// https://github.com/DefinitelyTyped/DefinitelyTyped/issues/20544 | ||
return props.content as ReactElement; | ||
return content as ReactElement; | ||
} | ||
|
||
// Otherwise, render a hidden copy of the children so that we can build the collection before constructing the state. | ||
// This should always come before the real DOM content so we have built the collection by the time it renders during SSR. | ||
|
||
// This is fine. CollectionDocumentContext never changes after mounting. | ||
// eslint-disable-next-line react-hooks/rules-of-hooks | ||
let {collection, document} = useCollectionDocument(props.createCollection); | ||
let state = useCollectionDocument(props.createCollection); | ||
let {collection, document} = ref.useCollectionDocument ? ref.useCollectionDocument(state) : state; | ||
let children = ref.useCollectionChildren ? ref.useCollectionChildren(props.children) : props.children; | ||
return ( | ||
<> | ||
<Hidden> | ||
<CollectionDocumentContext.Provider value={document}> | ||
{props.content} | ||
{content} | ||
</CollectionDocumentContext.Provider> | ||
</Hidden> | ||
<CollectionInner render={props.children} collection={collection} /> | ||
<CollectionInner render={children} collection={collection} /> | ||
</> | ||
); | ||
} | ||
|
@@ -94,7 +116,7 @@ const useSyncExternalStore = typeof React['useSyncExternalStore'] === 'function' | |
? React['useSyncExternalStore'] | ||
: useSyncExternalStoreFallback; | ||
|
||
function useCollectionDocument<T extends object, C extends BaseCollection<T>>(createCollection?: () => C): CollectionDocumentResult<T, C> { | ||
export function useCollectionDocument<T extends object, C extends BaseCollection<T>>(createCollection?: () => C): CollectionDocumentResult<T, C> { | ||
// The document instance is mutable, and should never change between renders. | ||
// useSyncExternalStore is used to subscribe to updates, which vends immutable Collection objects. | ||
let [document] = useState(() => new Document<T, C>(createCollection?.() || new BaseCollection() as C)); | ||
|
@@ -157,6 +179,12 @@ function useSSRCollectionNode<T extends Element>(Type: string, props: object, re | |
return <Type ref={itemRef}>{children}</Type>; | ||
} | ||
|
||
export function useCollectionRef<C extends BaseCollection<object>, E extends Element>(props: CollectionRenderProps<C>, ref: Ref<E>): CollectionRef<C, E> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naming and argument order is TBD. I was also considering |
||
let refObject = useObjectRef(ref) as CollectionRef<C, E>; | ||
|
||
return Object.assign(refObject, props); | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>) => ReactElement | null): (props: P & React.RefAttributes<E>) => ReactElement | null; | ||
export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>, node: Node<T>) => ReactElement | null): (props: P & React.RefAttributes<E>) => ReactElement | null; | ||
|
@@ -206,8 +234,6 @@ function useCollectionChildren<T extends object>(options: CachedChildrenOptions< | |
return useCachedChildren({...options, addIdAndValue: true}); | ||
} | ||
|
||
export interface CollectionProps<T> extends CachedChildrenOptions<T> {} | ||
|
||
const CollectionContext = createContext<CachedChildrenOptions<unknown> | null>(null); | ||
|
||
/** A Collection renders a list of items, automatically managing caching and keys. */ | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let me know if you’re okay with exporting this. Our use case only requires one-way sync, so we wouldn’t necessarily need it, but I feel that a true sync might be preferable, in which case this would be required.