Skip to content

feat(react-virtual): compatibility with the React compiler - #851

Closed
gdorsi wants to merge 4 commits into
TanStack:mainfrom
gdorsi:main
Closed

feat(react-virtual): compatibility with the React compiler#851
gdorsi wants to merge 4 commits into
TanStack:mainfrom
gdorsi:main

Conversation

@gdorsi

@gdorsi gdorsi commented Oct 10, 2024

Copy link
Copy Markdown

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.

Comment thread packages/react-virtual/src/index.tsx Outdated
})

return instance
return React.useMemo(() => Object.create(instance), [instance.getVirtualItems()])

Copy link
Copy Markdown
Author

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.

@BjoernRave

BjoernRave commented Nov 15, 2024

Copy link
Copy Markdown

looking forward for this to get merged, especially since with react 19 rc1 things seem to start moving again

@piecyk

piecyk commented Dec 12, 2024

Copy link
Copy Markdown
Collaborator

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

@gdorsi

gdorsi commented Dec 21, 2024

Copy link
Copy Markdown
Author

@piecyk

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)
    }
  }

@lachlancollins lachlancollins changed the title feat(react): compatibility with the React compiler feat(react-virtual): compatibility with the React compiler Mar 8, 2025
@changeset-bot

changeset-bot Bot commented Mar 8, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 51e3816

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@nx-cloud

nx-cloud Bot commented Mar 8, 2025

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 51e3816.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 28s View ↗
nx run-many --target=build --exclude=examples/** ✅ Succeeded 3s View ↗

☁️ Nx Cloud last updated this comment at 2025-03-08 01:14:44 UTC

@pkg-pr-new

pkg-pr-new Bot commented Mar 8, 2025

Copy link
Copy Markdown

Open in Stackblitz

More templates

@tanstack/angular-virtual

npm i https://pkg.pr.new/@tanstack/angular-virtual@851

@tanstack/lit-virtual

npm i https://pkg.pr.new/@tanstack/lit-virtual@851

@tanstack/solid-virtual

npm i https://pkg.pr.new/@tanstack/solid-virtual@851

@tanstack/react-virtual

npm i https://pkg.pr.new/@tanstack/react-virtual@851

@tanstack/virtual-core

npm i https://pkg.pr.new/@tanstack/virtual-core@851

@tanstack/svelte-virtual

npm i https://pkg.pr.new/@tanstack/svelte-virtual@851

@tanstack/vue-virtual

npm i https://pkg.pr.new/@tanstack/vue-virtual@851

commit: 51e3816

@The-Best-Codes

Copy link
Copy Markdown

Please merge this, it would be awesome!

@samesfahani-tuplehealth

Copy link
Copy Markdown

Any timeline on this PR getting merged?

@gdorsi

gdorsi commented May 2, 2025

Copy link
Copy Markdown
Author

This is not really a fully featured fix, more like a POC.

Going to close to avoid unnecessary distractions.

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.

React 19 issue with useWindowVirtualizer React Compiler: useVirtual results are cached at their initial values

6 participants