Skip to content

Conversation

@cngonzalez
Copy link
Member

@cngonzalez cngonzalez commented Nov 14, 2025

Description

useDocumentProjection was actually a bit finicky about the projection being passed to it. We had a ticket about it looping forever

My best guess about what was happening was this:

  1. Any helper functions, or even template literals, created new references passed to useDocumentProjection
  2. useDocumentProjection would then call getProjectionState. It always returns a new StateSource, which returns a new subscribe function
  3. useSyncExternalStore receives the new subscribe function, triggering a re-render
  4. re-render causes the loop to start all over again

This PR adds some memoization to maintain consistency in projection strings and state sources.

We didn't catch this in casual clicking around because defineProjection was doing some of the memoization work for us, I think. To address this, I added some common ways people would use useDocumentProjection and also some e2e tests against it.

What to review

Any risks of over memoization or staleness?

Testing

e2e tests (I think this is hard to capture in React tests and we still have 100% coverage).

To see the OLD behavior,

  1. git fetch
  2. git checkout main
  3. git checkout origin/fix/sdk-660/ensure-doc-projections-reuse-state-source apps/kitchensink-react/src/DocumentCollection/DocumentProjectionRoute.tsx
  4. pnpm dev
  5. go to https://www.sanity.io/@oblZgbTFj?dev=http%3A%2F%2Flocalhost%3A3333
  6. go to https://www.sanity.io/@oblZgbTFj/application/__dev/document-projection
  7. Click around some of the many buttons, you'll see things hanging.

This PR fixes that.

@vercel
Copy link

vercel bot commented Nov 14, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
sdk-docs Ready Ready Preview Comment Nov 14, 2025 7:20pm
sdk-kitchensink-react Ready Ready Preview Comment Nov 14, 2025 7:20pm

Copy link
Member Author

cngonzalez commented Nov 14, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Member

@ryanbonial ryanbonial left a comment

Choose a reason for hiding this comment

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

Fix works well. Thanks!

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.

2 participants