Feat/add use async memo suspense #6
Conversation
Adds JSDocs, README documentation, and tests for the new `useAsyncMemoSuspense` hook. The documentation includes examples and important notes about SSR and client component usage. The tests cover suspense, error handling, dependency changes, and scope usage.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new React hook, Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useAsyncMemoSuspense
participant Cache
participant Promise
Component->>useAsyncMemoSuspense: Call hook with factory, deps, options
useAsyncMemoSuspense->>Cache: Check for cached entry (by factory+deps+scope)
alt Cache hit and resolved
useAsyncMemoSuspense-->>Component: Return cached result
else Cache hit and pending
useAsyncMemoSuspense-->>Component: Throw pending promise (suspend)
else Cache hit and error
useAsyncMemoSuspense-->>Component: Throw error
else Cache miss or deps changed
useAsyncMemoSuspense->>Promise: Call factory (async or sync)
Promise-->>useAsyncMemoSuspense: Resolves or rejects
useAsyncMemoSuspense->>Cache: Store result or error
useAsyncMemoSuspense-->>Component: Throw pending promise (suspend)
end
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 review |
✅ Actions performedReview triggered.
|
![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 (1)
src/useAsyncMemoSuspense.ts (1)
20-28: Consider cache key generation robustness.The current cache key generation using
factory.toString()may be fragile in production environments where code is minified or when functions are dynamically generated.Consider these alternatives for more robust cache key generation:
function getCacheKey( factory: () => Promise<unknown> | unknown, deps: DependencyList, scope?: string ): string { - // Use function toString and JSON.stringify for deps as a simple cache key - // In production, you might want a more sophisticated key generation - return JSON.stringify([factory.toString(), deps, scope || ""]); + // Use a WeakMap to assign unique IDs to functions for more reliable caching + const factoryId = getFunctionId(factory); + return JSON.stringify([factoryId, deps, scope || ""]); }However, the current approach is acceptable for most use cases and is clearly documented as simple.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
README.md(3 hunks)package.json(1 hunks)src/__tests__/useAsyncMemo.test.ts(1 hunks)src/__tests__/useAsyncMemoSuspense.test.tsx(1 hunks)src/index.ts(1 hunks)src/useAsyncMemoSuspense.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: davemecha
PR: davemecha/use-async-effekt#3
File: src/__tests__/test-utils.ts:6-8
Timestamp: 2025-06-29T11:06:37.033Z
Learning: In matrix testing scenarios where different versions of testing libraries are dynamically installed (like @testing-library/react-hooks vs @testing-library/react across React versions), using `any` types may be necessary due to unstable type dependencies, rather than importing specific types that may not be available in all test configurations.
package.json (1)
Learnt from: davemecha
PR: davemecha/use-async-effekt#3
File: src/__tests__/test-utils.ts:6-8
Timestamp: 2025-06-29T11:06:37.033Z
Learning: In matrix testing scenarios where different versions of testing libraries are dynamically installed (like @testing-library/react-hooks vs @testing-library/react across React versions), using `any` types may be necessary due to unstable type dependencies, rather than importing specific types that may not be available in all test configurations.
src/__tests__/useAsyncMemo.test.ts (1)
Learnt from: davemecha
PR: davemecha/use-async-effekt#3
File: src/__tests__/test-utils.ts:6-8
Timestamp: 2025-06-29T11:06:37.033Z
Learning: In matrix testing scenarios where different versions of testing libraries are dynamically installed (like @testing-library/react-hooks vs @testing-library/react across React versions), using `any` types may be necessary due to unstable type dependencies, rather than importing specific types that may not be available in all test configurations.
README.md (1)
Learnt from: davemecha
PR: davemecha/use-async-effekt#3
File: src/__tests__/test-utils.ts:6-8
Timestamp: 2025-06-29T11:06:37.033Z
Learning: In matrix testing scenarios where different versions of testing libraries are dynamically installed (like @testing-library/react-hooks vs @testing-library/react across React versions), using `any` types may be necessary due to unstable type dependencies, rather than importing specific types that may not be available in all test configurations.
src/useAsyncMemoSuspense.ts (1)
Learnt from: davemecha
PR: davemecha/use-async-effekt#3
File: src/__tests__/test-utils.ts:6-8
Timestamp: 2025-06-29T11:06:37.033Z
Learning: In matrix testing scenarios where different versions of testing libraries are dynamically installed (like @testing-library/react-hooks vs @testing-library/react across React versions), using `any` types may be necessary due to unstable type dependencies, rather than importing specific types that may not be available in all test configurations.
src/__tests__/useAsyncMemoSuspense.test.tsx (1)
Learnt from: davemecha
PR: davemecha/use-async-effekt#3
File: src/__tests__/test-utils.ts:6-8
Timestamp: 2025-06-29T11:06:37.033Z
Learning: In matrix testing scenarios where different versions of testing libraries are dynamically installed (like @testing-library/react-hooks vs @testing-library/react across React versions), using `any` types may be necessary due to unstable type dependencies, rather than importing specific types that may not be available in all test configurations.
🪛 Biome (1.9.4)
src/__tests__/useAsyncMemoSuspense.test.tsx
[error] 64-64: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🪛 GitHub Actions: Test
src/__tests__/useAsyncMemoSuspense.test.tsx
[error] 341-341: TypeError: (0 , react_1.startTransition) is not a function. Test failure in 'useAsyncMemoSuspense - Core Functionality › React 18 concurrent features › should work with startTransition'.
🔇 Additional comments (13)
package.json (1)
63-63: LGTM! Dependency update supports new testing requirements.The ts-jest update from
^29.1.0to^29.4.0is a minor version bump that should include improvements and bug fixes while maintaining compatibility. This aligns well with the comprehensive test suite added for the newuseAsyncMemoSuspensehook.src/__tests__/useAsyncMemo.test.ts (1)
7-7: Excellent addition for test isolation.Adding
jest.clearAllMocks()ensures that mock state doesn't leak between tests, improving test reliability and preventing potential flaky test results. This is a best practice that enhances the overall test suite quality.src/index.ts (1)
3-3: Clean addition to the public API.The export follows the established pattern and correctly exposes the new
useAsyncMemoSuspensehook to library consumers.README.md (2)
313-313: Correct ESLint configuration update.The addition of
useAsyncMemoSuspenseto theadditionalHookspattern ensures proper dependency array linting for the new hook, maintaining consistency with existing hooks.Also applies to: 328-328
363-407: Excellent comprehensive documentation.The documentation for
useAsyncMemoSuspenseis thorough and well-structured:
- Clear API reference with parameter descriptions
- Important SSR considerations are prominently highlighted
- Experimental status is clearly marked
- Practical example demonstrates proper Suspense integration
- The scope parameter usage is well explained
This provides developers with everything they need to use the hook correctly.
src/useAsyncMemoSuspense.ts (7)
3-7: Well-designed cache entry type system.The union type for
CacheEntryclearly models the three possible states (pending, success, error) with appropriate data for each state. Theundefinedoption handles the initial case cleanly.
9-10: Correct dependency comparison implementation.Using
Object.is()for dependency comparison is the right choice as it handles edge cases likeNaNand-0correctly, matching React's own dependency comparison logic.
84-85: Clever client component enforcement.Using
useRef(undefined)to force the component to be a client component is a creative approach that leverages React's hook rules without any side effects.
87-88: Proper SSR handling.Returning
undefinedon the server side ensures proper Suspense fallback behavior during hydration, preventing hydration mismatches while allowing the client to suspend appropriately.
94-112: Solid cache management and async handling.The cache invalidation logic correctly handles dependency changes, and the promise handling properly updates the cache entry in place using
Object.assign. The use ofPromise.resolve()ensures both sync and async factories are handled uniformly.
114-116: Correct Suspense integration.The return logic properly implements the Suspense pattern:
- Returns the result when available
- Throws errors to trigger error boundaries
- Throws promises to suspend rendering
This follows React Suspense conventions perfectly.
30-78: Comprehensive and accurate JSDoc documentation.The documentation clearly explains:
- Experimental status
- SSR behavior and client component requirement
- Usage patterns with practical examples
- Integration with Suspense boundaries
This will help developers understand the hook's behavior and constraints.
src/__tests__/useAsyncMemoSuspense.test.tsx (1)
298-365: ImportstartTransitionfrom React to fix the test failureThe project’s package.json shows React ^18.0.0, so
startTransitionis available—but it isn’t in scope in your test. Add the import at the top of the file instead of conditionally skipping the test.Locations to update:
- src/tests/useAsyncMemoSuspense.test.tsx (near the existing React imports)
Suggested diff:
--- a/src/__tests__/useAsyncMemoSuspense.test.tsx +++ b/src/__tests__/useAsyncMemoSuspense.test.tsx @@ 1,5 +1,6 @@ -import React from 'react'; +import React, { startTransition } from 'react'; import { render, screen, act, waitFor } from '@testing-library/react'; import rtlAct from 'react-test-renderer/src/act'; import { Suspense, startTransition } from 'react';(If you already import
Reactand other hooks, just includestartTransitionin that import.)After this change, the “should work with startTransition” test will run as intended under React 18.
Likely an incorrect or invalid review comment.
|
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
…ithChildren syntax
2 participants
Summary by CodeRabbit