refactor(EditableDetails): optimize form handling with useMemo #3048
Conversation
- Replace direct defaultValues assignment with memoized formValues - Remove unnecessary useEffect for form reset on data change - Simplify conditional rendering logic in the return statement
|
WalkthroughThe EditableDetails component now uses the Changes
Possibly related PRs
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx (1)
129-135: Add null check and optimize object creation.While the memoization is good, consider these improvements:
- Add null check for data prop
- Use Object.fromEntries for better performance
const formValues = useMemo(() => { - return data?.reduce((acc, curr) => { - acc[curr.title] = curr.value; - - return acc; - }, {}); + if (!data) return {}; + return Object.fromEntries(data.map(item => [item.title, item.value])); }, [data]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: lint
🔇 Additional comments (4)
apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx (4)
11-11: LGTM!Clean addition of the useMemo hook import.
137-139: LGTM!Using
valuesinstead ofdefaultValuesis the correct choice here as it ensures the form stays synchronized with external updates.
282-288: LGTM!The displayValue function changes improve code readability while maintaining the same logic.
303-305: LGTM!The conditional rendering changes improve code readability by using proper braces and maintaining a clean early return pattern.
defaultValues alone:
const form = useForm({ defaultValues })Only sets the initial values when the form is first mounted
Won't respond to changes in defaultValues on subsequent renders
Think of it as "what should the form start with?"
values alone:
const form = useForm({ values: defaultValues })Updates form values whenever defaultValues changes
Overwrites any user input when defaultValues changes
Think of it as "what should the form currently show?"
just use
values: defaultValueswould be sufficient since we want the form to stay synchronized with external updates.Summary by CodeRabbit