Skip to content

Comments

useGridDimensions: useSyncExternalStore implementation#3968

Open
nstepien wants to merge 6 commits intomainfrom
externaldimensions
Open

useGridDimensions: useSyncExternalStore implementation#3968
nstepien wants to merge 6 commits intomainfrom
externaldimensions

Conversation

@nstepien
Copy link
Collaborator

@nstepien nstepien commented Feb 22, 2026

useSyncExternalStore seems like the better choice to handle this, especially when concurrent rendering is involved.

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.58%. Comparing base (dd8eb88) to head (f5623d4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3968      +/-   ##
==========================================
+ Coverage   97.53%   97.58%   +0.04%     
==========================================
  Files          36       36              
  Lines        1463     1492      +29     
  Branches      472      478       +6     
==========================================
+ Hits         1427     1456      +29     
  Misses         36       36              
Files with missing lines Coverage Δ
src/hooks/useGridDimensions.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

params[0] instanceof Error &&
params[0].message === 'ResizeObserver loop completed with undelivered notifications.'
) {
return;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just hoping this will be resolved 🤞

// don't break in Node.js (SSR), jsdom, and environments that don't support ResizeObserver
const resizeObserver =
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
globalThis.ResizeObserver == null ? null : new ResizeObserver(resizeObserverCallback);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We'll have 1 RO instance for all rendered grids on the page, so all resize events will be batched together -> improved perf?

const idToTargetMap = new Map<string, HTMLDivElement>();
// use an unmanaged WeakMap so we preserve the cache even when
// the component partially unmounts via Suspense or Activity
const sizeMap = new WeakMap<HTMLDivElement, ResizeObserverSize>();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took great care to ensure this works great even if the components unmounts but the div isn't deleted.
The updated hook is more complicated than before, but should behave great with concurrent rendering ✌️

}

export function useGridDimensions() {
const id = useId();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using an id to sync things.
I tried using gridRef.current at first, but getSnapshot gets called during render, so it would probably not be safe.

}
}
return initialSize;
}, [id]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getSnapshot gets called multiple times during render, so it has to be as fast as possible, and must avoid returning different values.

@nstepien nstepien marked this pull request as ready for review February 25, 2026 00:20
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