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 21, 2025 1:20pm
sdk-kitchensink-react Ready Ready Preview Comment Nov 21, 2025 1:20pm

Copy link
Member Author

cngonzalez commented Nov 14, 2025

ryanbonial
ryanbonial previously approved these changes Nov 14, 2025
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!

@cngonzalez cngonzalez force-pushed the fix/sdk-660/ensure-doc-projections-reuse-state-source branch from aeff275 to e1310c2 Compare November 18, 2025 13:37
@cngonzalez cngonzalez force-pushed the fix/update-e2e-tests-for-running-dashboard branch from f9dba71 to 95755bf Compare November 18, 2025 13:37
@cngonzalez cngonzalez force-pushed the fix/sdk-660/ensure-doc-projections-reuse-state-source branch from e1310c2 to 6f60f63 Compare November 18, 2025 13:43
@cngonzalez cngonzalez force-pushed the fix/update-e2e-tests-for-running-dashboard branch from 95755bf to cd12fbe Compare November 18, 2025 13:43
@cngonzalez cngonzalez force-pushed the fix/sdk-660/ensure-doc-projections-reuse-state-source branch from 6f60f63 to 9fbbb0e Compare November 18, 2025 17:53
@cngonzalez cngonzalez force-pushed the fix/update-e2e-tests-for-running-dashboard branch from cd12fbe to 71274df Compare November 18, 2025 17:53
@cngonzalez cngonzalez force-pushed the fix/sdk-660/ensure-doc-projections-reuse-state-source branch from 9fbbb0e to 74d86d1 Compare November 20, 2025 13:12
@cngonzalez cngonzalez force-pushed the fix/sdk-660/ensure-doc-projections-reuse-state-source branch from 74d86d1 to 9baabcc Compare November 20, 2025 13:18
@cngonzalez cngonzalez force-pushed the fix/update-e2e-tests-for-running-dashboard branch from 2df6371 to 6667598 Compare November 20, 2025 13:18
@cngonzalez cngonzalez changed the base branch from fix/update-e2e-tests-for-running-dashboard to graphite-base/669 November 21, 2025 13:18
@cngonzalez cngonzalez force-pushed the fix/sdk-660/ensure-doc-projections-reuse-state-source branch from 9baabcc to 176e4ef Compare November 21, 2025 13:18
@graphite-app graphite-app bot changed the base branch from graphite-base/669 to main November 21, 2025 13:19
@graphite-app graphite-app bot dismissed ryanbonial’s stale review November 21, 2025 13:19

The base branch was changed.

@cngonzalez cngonzalez merged commit 474ff5d into main Nov 21, 2025
19 checks passed
@cngonzalez cngonzalez deleted the fix/sdk-660/ensure-doc-projections-reuse-state-source branch November 21, 2025 15:12
@squiggler squiggler bot mentioned this pull request Nov 21, 2025
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