-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
feat(react): compatibility with the React compiler #851
base: main
Are you sure you want to change the base?
Conversation
@@ -50,7 +50,7 @@ function useVirtualizerBase< | |||
return instance._willUpdate() | |||
}) | |||
|
|||
return instance | |||
return React.useMemo(() => Object.create(instance), [instance.getVirtualItems()]) |
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.
We could move this function inside the virtual core and use the memo
helper to invalidate the instance generation.
Should give us more flexibility and will be easier to cover more invalidations.
looking forward for this to get merged, especially since with react 19 rc1 things seem to start moving again |
Thinking about this, if we want to move in this direction, useSyncExternalStore feels like a more natural fit compared to useMemo. We can make it as separate hook" For now 'use no memo' seems like a reasonable temporary workaround! https://react.dev/learn/react-compiler#something-is-not-working-after-compilation |
Would somthing like this work for you? function useMemoSafeVirtualizer<
TScrollElement extends Element | Window,
TItemElement extends Element,
>(
options: VirtualizerOptions<TScrollElement, TItemElement>,
): Virtualizer<TScrollElement, TItemElement> {
const [instance] = React.useState(
() => new Virtualizer<TScrollElement, TItemElement>(options),
)
instance.setOptions(options)
React.useEffect(() => {
return instance._didMount()
}, [])
useIsomorphicLayoutEffect(() => {
return instance._willUpdate()
})
return useSyncExternalStore(
useCallback((onStoreChange) => instance.subscribe(onStoreChange), [instance]),
() => instance.getMemoSafeInstance(),
() => instance.getMemoSafeInstance(),
)
} We would need to add subscribe and getMemoSafeInstance to the core: getMemoSafeInstance = memo(
() => [this.getVirtualItems()],
() => Object.create(this),
{
key: process.env.NODE_ENV !== 'production' && 'getMemoSafeInstance',
debug: () => this.options.debug,
},
)
private notify = (sync: boolean) => {
this.options.onChange?.(this, sync)
this.subscribers.forEach(fn => fn(this, sync));
}
subscribe = (callback) => {
this.subscribers.add(callback)
retrurn () => {
this.subscribers.delete(callback)
}
} |
Fixes #736
Fixes #743
This PR addresses a critical issue with useVirtualizer compatibility when used alongside the React compiler. Previously, the React compiler would cache results based on the virtualizer instance, preventing updates when virtual items changed.
The solution implements a wrapper using Object.create around the virtualizer instance.
This approach maintains the internal state while allowing the instance reference to change.
The internal state remains constant across updates, with only the instance reference being modified when virtual items change.
This change affects object property behavior. Spread operations and Object.keys() on the virtualizer now return empty results. This impacts code relying on these operations and could be considered a breaking change.
If this, or the
getVirtualItems
eager evaluation, is a cause of concern we could make the transformation opt-in via option flag or could even be implemented externally as a separate hook.