-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactor collection internals in preparation for public API #6608
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. So just for book keeping, the extra Collection stuff we are planning on exposing from a new package is as follows:
with those (and the already exposed Collection), hooks users can use new collections for their components 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. Yeah that sounds right. Not sure what to do about BaseCollection which is used to subclass in TableCollection too. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,10 +11,10 @@ | |
*/ | ||
import {CollectionBase, DropTargetDelegate, Key, LayoutDelegate} from '@react-types/shared'; | ||
import {createPortal} from 'react-dom'; | ||
import {forwardRefType, StyleProps} from './utils'; | ||
import {forwardRefType, Hidden, StyleProps} from './utils'; | ||
import {Collection as ICollection, Node, SelectionBehavior, SelectionMode, SectionProps as SharedSectionProps} from 'react-stately'; | ||
import {mergeProps, useIsSSR} from 'react-aria'; | ||
import React, {cloneElement, createContext, ForwardedRef, forwardRef, HTMLAttributes, JSX, ReactElement, ReactNode, RefObject, useCallback, useContext, useMemo, useRef} from 'react'; | ||
import React, {cloneElement, createContext, ForwardedRef, forwardRef, HTMLAttributes, JSX, ReactElement, ReactNode, RefObject, useCallback, useContext, useMemo, useRef, useState} from 'react'; | ||
import {useLayoutEffect} from '@react-aria/utils'; | ||
import {useSyncExternalStore as useSyncExternalStoreShim} from 'use-sync-external-store/shim/index.js'; | ||
|
||
|
@@ -274,7 +274,7 @@ class BaseNode<T> { | |
* A mutable element node in the fake DOM tree. It owns an immutable | ||
* Collection Node which is copied on write. | ||
*/ | ||
export class ElementNode<T> extends BaseNode<T> { | ||
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; | ||
|
@@ -503,7 +503,7 @@ export class BaseCollection<T> implements ICollection<Node<T>> { | |
* A mutable Document in the fake DOM. It owns an immutable Collection instance, | ||
* which is lazily copied on write during updates. | ||
*/ | ||
export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extends BaseNode<T> { | ||
class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extends BaseNode<T> { | ||
nodeType = 11; // DOCUMENT_FRAGMENT_NODE | ||
ownerDocument = this; | ||
dirtyNodes: Set<BaseNode<T>> = new Set(); | ||
|
@@ -709,16 +709,41 @@ export function useCollectionChildren<T extends object>(props: CachedChildrenOpt | |
} | ||
|
||
const ShallowRenderContext = createContext(false); | ||
const CollectionDocumentContext = createContext<Document<any, BaseCollection<any>> | null>(null); | ||
|
||
interface CollectionResult<C> { | ||
portal: ReactNode, | ||
collection: C | ||
export interface CollectionBuilderProps<C extends BaseCollection<object>> { | ||
content: ReactNode, | ||
children: (collection: C) => ReactNode, | ||
createCollection?: () => C | ||
} | ||
|
||
export function useCollection<T extends object, C extends BaseCollection<T>>(props: CollectionProps<T>, initialCollection?: C): CollectionResult<C> { | ||
let {collection, document} = useCollectionDocument<T, C>(initialCollection); | ||
let portal = useCollectionPortal<T, C>(props, document); | ||
return {portal, collection}; | ||
export function CollectionBuilder<C extends BaseCollection<object>>(props: CollectionBuilderProps<C>) { | ||
// If a document was provided above us, we're already in a hidden tree. Just render the content. | ||
let doc = useContext(CollectionDocumentContext); | ||
if (doc) { | ||
return props.content; | ||
} | ||
|
||
// 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); | ||
return ( | ||
<> | ||
<Hidden> | ||
<CollectionDocumentContext.Provider value={document}> | ||
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. These are technically not required for collections that render collection children directly (like Menu), it's only needed for cases like Select and ComboBox that have intermediary elements. It shouldn't hurt anything but if there were performance issues with this we could theoretically optimize it (e.g. check if |
||
{props.content} | ||
</CollectionDocumentContext.Provider> | ||
</Hidden> | ||
<CollectionInner render={props.children} collection={collection} /> | ||
</> | ||
); | ||
} | ||
|
||
function CollectionInner({collection, render}) { | ||
return render(collection); | ||
} | ||
|
||
interface CollectionDocumentResult<T, C extends BaseCollection<T>> { | ||
|
@@ -747,10 +772,10 @@ const useSyncExternalStore = typeof React['useSyncExternalStore'] === 'function' | |
? React['useSyncExternalStore'] | ||
: useSyncExternalStoreFallback; | ||
|
||
export function useCollectionDocument<T extends object, C extends BaseCollection<T>>(initialCollection?: C): CollectionDocumentResult<T, C> { | ||
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 = useMemo(() => new Document<T, C>(initialCollection || new BaseCollection() as C), [initialCollection]); | ||
let [document] = useState(() => new Document<T, C>(createCollection?.() || new BaseCollection() as C)); | ||
let subscribe = useCallback((fn: () => void) => document.subscribe(fn), [document]); | ||
let getSnapshot = useCallback(() => { | ||
let collection = document.getCollection(); | ||
|
@@ -779,27 +804,6 @@ export function useCollectionDocument<T extends object, C extends BaseCollection | |
} | ||
|
||
const SSRContext = createContext<BaseNode<any> | null>(null); | ||
export const CollectionDocumentContext = createContext<Document<any, BaseCollection<any>> | null>(null); | ||
|
||
export function useCollectionPortal<T extends object, C extends BaseCollection<T>>(props: CollectionProps<T>, document?: Document<T, C>): ReactNode { | ||
let ctx = useContext(CollectionDocumentContext); | ||
let doc = document ?? ctx!; | ||
let children = useCollectionChildren(props); | ||
let wrappedChildren = useMemo(() => ( | ||
<ShallowRenderContext.Provider value> | ||
{children} | ||
</ShallowRenderContext.Provider> | ||
), [children]); | ||
// During SSR, we render the content directly, and append nodes to the document during render. | ||
// The collection children return null so that nothing is actually rendered into the HTML. | ||
return useIsSSR() | ||
? <SSRContext.Provider value={doc}>{wrappedChildren}</SSRContext.Provider> | ||
: createPortal(wrappedChildren, doc as unknown as Element); | ||
} | ||
|
||
export function CollectionPortal<T extends object>(props: CollectionProps<T>) { | ||
return <>{useCollectionPortal(props)}</>; | ||
} | ||
|
||
export interface ItemRenderProps { | ||
/** | ||
|
@@ -919,7 +923,30 @@ export function Collection<T extends object>(props: CollectionProps<T>): JSX.Ele | |
let ctx = useContext(CollectionContext)!; | ||
props = mergeProps(ctx, props); | ||
props.dependencies = (ctx?.dependencies || []).concat(props.dependencies); | ||
return <>{useCollectionChildren(props)}</>; | ||
let children = useCollectionChildren(props); | ||
|
||
let doc = useContext(CollectionDocumentContext); | ||
if (doc) { | ||
return <CollectionRoot>{children}</CollectionRoot>; | ||
} | ||
|
||
return <>{children}</>; | ||
} | ||
|
||
function CollectionRoot({children}) { | ||
let doc = useContext(CollectionDocumentContext); | ||
let wrappedChildren = useMemo(() => ( | ||
<CollectionDocumentContext.Provider value={null}> | ||
<ShallowRenderContext.Provider value> | ||
{children} | ||
</ShallowRenderContext.Provider> | ||
</CollectionDocumentContext.Provider> | ||
), [children]); | ||
// During SSR, we render the content directly, and append nodes to the document during render. | ||
// The collection children return null so that nothing is actually rendered into the HTML. | ||
return useIsSSR() | ||
? <SSRContext.Provider value={doc}>{wrappedChildren}</SSRContext.Provider> | ||
: createPortal(wrappedChildren, doc as unknown as Element); | ||
} | ||
|
||
export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>) => JSX.Element): (props: P & React.RefAttributes<T>) => React.ReactElement | null; | ||
|
@@ -976,7 +1003,7 @@ export interface CollectionRenderer { | |
CollectionBranch: React.ComponentType<CollectionBranchProps> | ||
} | ||
|
||
const DefaultCollectionRenderer: CollectionRenderer = { | ||
export const DefaultCollectionRenderer: CollectionRenderer = { | ||
CollectionRoot({collection}) { | ||
return useCachedChildren({ | ||
items: collection, | ||
|
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.
Don't love this, but we need to avoid a circular dependency in RAC
Table
. We need the selection state in order to determine whether to render checkboxes based on the selectionBehavior etc (viauseTableOptions
), but we can't build the fullTableState
until we have the collection. This prop lets us split this up to calluseMultipleSelectionState
directly, and pass this state down intouseTableState
. It is unsafe because of the rules of hooks below, so this is not meant to be a public API. Happy to change if someone has a better idea.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.
Would it make more sense for useMultipleSelectionState to have this and reconcile the difference? then if we run into this issue in other places it's not just in useGridState?
Also, we could not call it conditionally below, it just sets up state and then if nothing interacts with it, it won't do anything. So should be harmless to call it unconditionally and only use the result if props.UNSAFE_selectionState isn't passed in.
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.
yeah maybe. It bothers me to do extra unnecessary work and throw it away lol