Conversation
1 similar comment
There was a problem hiding this comment.
Pull request overview
This PR moves “stable identity” memoization for cache value/error from the React hook layer (useCache) down into the core Cache state updates, aiming to reduce unnecessary downstream updates when newly produced values are deeply equal to previous ones.
Changes:
- Removed
useMemoEquals-based memoization ofvalue/errorinsideuseCache. - Added
deepEqual-based reference reuse insideCache.watchPromise()when updatingthis.state, so equalvalue/errorkeep prior object identity.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/react/useCache.ts |
Removes hook-level memoization of returned value/error. |
src/core/cache.ts |
Adds core-level memoization to reuse previous value/error references when deeply equal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Memoize value and error to keep their identity stable if they're equal | ||
| if ( | ||
| newState.status === 'value' && | ||
| state.status === 'value' && | ||
| deepEqual(state.value, newState.value) | ||
| ) { |
There was a problem hiding this comment.
deepEqual is now executed inside the cache update path (including the PromiseWithState fast-path). For large cached values this can add noticeable CPU overhead even when there are no consumers (no one subscribed to this.state). Consider guarding the deep-equality memoization behind a check like whether this.state has subscribers/needs identity stability, or centralizing the memoization so the equality check is only performed when it will actually prevent downstream work (e.g., listener notifications / React rerenders).
| this.state.set((state) => { | ||
| const newValue = | ||
| state.status === 'value' && deepEqual(state.value, value) ? state.value : value; | ||
| return { | ||
| status: 'value', |
There was a problem hiding this comment.
The value/error identity memoization logic is duplicated across the resolved-PromiseWithState branch and the normal resolve/error branches. Consider extracting a small helper (e.g., “reusePreviousIfEqual”) to avoid future drift if this logic needs to change.
No description provided.