fix(use-image): cached image flickering issue #4442
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 1239c07 The changes in this PR will be included in the next version bump. This PR includes changesets to release 50 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request introduces a new utility hook Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/hooks/use-image/__tests__/use-image.test.tsx (1)
37-43: New test coverage for cached images
This test effectively confirms that the hook resolves the appropriate status when the image is already loaded (cached). Consider adding an SSR-focused test if feasible, to ensure hydration behavior is validated in the test suite..changeset/small-kids-walk.md (1)
1-5: Concise changeset entry
The summary succinctly documents the addition ofuseIsHydrated. If you wish to also mention that it addresses cached image flickering, consider including a brief note. Otherwise, this is acceptable as-is.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/small-kids-walk.md(1 hunks).changeset/tame-glasses-press.md(1 hunks)packages/hooks/use-image/__tests__/use-image.test.tsx(1 hunks)packages/hooks/use-image/package.json(1 hunks)packages/hooks/use-image/src/index.ts(3 hunks)packages/utilities/react-utils/src/index.ts(1 hunks)packages/utilities/react-utils/src/use-is-hydrated.ts(1 hunks)
🔇 Additional comments (8)
packages/hooks/use-image/src/index.ts (4)
7-8: Properly imported dependencies
These imports correctly introduce the necessary React hooks as well as the useIsHydrated utility.
106-130: Image setup and initial status calculation
The helper function setImageAndGetInitialStatus responsibly handles image attributes (such as crossOrigin, srcSet, etc.). This is well-designed and clear. Keep in mind that if more image properties are introduced, they should also be set here for consistency.
132-136: Informative fallback-check helper
shouldShowFallbackImage and its usage of fallbackStrategy are straightforward and enhance code readability.
68-76: Conditionally creating the Image object based on hydration
This logic correctly defers image instantiation until hydration occurs, mitigating server-side rendering conflicts. However, please verify if any race condition could arise when isHydrated transitions from false to true.
✅ Verification successful
Let me gather more information about the implementation of useIsHydrated and the image loading logic.
Let me check the effect hooks and status updates in the image component.
No race conditions found in hydration-based image instantiation
The implementation is safe from race conditions for the following reasons:
useIsHydratedusesuseSyncExternalStorewhich guarantees consistent hydration state transitions- The initial image state is properly set during component initialization
- The
useEffecthook properly handles cleanup through theflushfunction - Image loading events are correctly managed regardless of when hydration occurs
The code correctly handles both the server-side rendering phase (where isHydrated is false) and the client-side hydration phase, with proper cleanup and state management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for all usage of `isHydrated` to ensure consistent updates.
rg -A 3 "isHydrated"
Length of output: 1081
Script:
#!/bin/bash
# Check the implementation of useIsHydrated
cat packages/utilities/react-utils/src/use-is-hydrated.ts
# Check the implementation of setImageAndGetInitialStatus and related effects
ast-grep --pattern 'function setImageAndGetInitialStatus($_) { $$$ }'
Length of output: 743
Script:
#!/bin/bash
# Check the implementation of useEffect hooks and status management
ast-grep --pattern 'useEffect(() => { $$$ })'
# Check the full implementation of the image hook
cat packages/hooks/use-image/src/index.ts
Length of output: 3774
packages/utilities/react-utils/src/use-is-hydrated.ts (1)
1-30: Efficient use of useSyncExternalStore
This hook leverages useSyncExternalStore properly and returns a stable hydration state. The no-op subscription strategy is appropriate, given that you do not expect external triggers.
packages/utilities/react-utils/src/index.ts (1)
36-36: Export for useIsHydrated
Exposing useIsHydrated aligns well with the rest of the utilities and maintains consistency in the package’s index.
.changeset/tame-glasses-press.md (1)
1-5: LGTM! Changeset correctly documents the patch.
The changeset properly indicates a patch version bump for the use-image package and references the relevant issue #4271.
packages/hooks/use-image/package.json (1)
37-38: LGTM! Dependencies are properly configured.
The addition of @nextui-org/react-utils as a workspace dependency is correct and follows the existing pattern. This dependency is required for the new hydration check functionality.
Let's verify the usage of the new dependency:
✅ Verification successful
The dependency is correctly used and properly imported
The @nextui-org/react-utils dependency is being used correctly in the package. The useIsHydrated hook is imported from this package and is used within the useImage hook to handle hydration-aware image loading. The implementation shows proper integration with the hydration check functionality.
Key usage points:
- Imported at the top:
import {useIsHydrated} from "@nextui-org/react-utils" - Used to initialize the image ref:
const isHydrated = useIsHydrated() - Affects the initial state and image creation logic based on hydration status
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the import and usage of useIsHydrated from react-utils
# Expected: Find imports of useIsHydrated in the use-image package
rg -t ts -t tsx "useIsHydrated|@nextui-org/react-utils" packages/hooks/use-image/
Length of output: 111
Script:
#!/bin/bash
# Let's try a different approach to find the imports and usage
# First, let's check all TypeScript files in the use-image package
fd '\.(ts|tsx)$' packages/hooks/use-image/ --exec rg -l "@nextui-org/react-utils|useIsHydrated" {}
# Let's also check the package contents
cat packages/hooks/use-image/src/use-image.ts
Length of output: 262
Script:
#!/bin/bash
# Let's check the content of index.ts since it matched our search
cat packages/hooks/use-image/src/index.ts
# Let's also check if there are any other source files in the package
fd '\.(ts|tsx)$' packages/hooks/use-image/src/
Length of output: 3813
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
pr4442-demo.webm
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
useIsHydratedhook for determining component hydration status.useImagehook with hydration checks to improve image loading behavior.Bug Fixes
use-imagepackage.Tests
useImagehook.Chores
use-imagepackage to include@nextui-org/react-utils.