fix: command modal mutation #6641
Conversation
prateekshourya29
requested review from
anmolsinghbhatia and
sriramveeraghanta
WalkthroughThe CommandModal component in the command-modal.tsx file has been updated to enhance the handling of issue details. The useIssueDetail hook is now destructured to include a new getIssueById function. This function is used to derive issue details from the workItemDetailsSWR data rather than using a direct assignment. Additionally, the variable naming has been revised along with a clarifying comment, making the data-fetching logic more explicit and structured. Changes
Sequence Diagram(s)sequenceDiagram
participant CM as CommandModal
participant Hook as useIssueDetail
participant SWR as workItemDetailsSWR
CM->>Hook: Call useIssueDetail()
Hook-->>CM: Return { issue: { getIssueById }, fetchIssueWithIdentifier }
CM->>Hook: Call getIssueById(issueId)
Hook-->>CM: Return issue details
Note over CM: Issue details derived from SWR via getIssueById
Possibly related PRs
Suggested labels
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)
web/core/components/command-palette/command-modal.tsx (1)
336-339: Consider adding loading state for issue details.While the implementation correctly uses the store data, it might be good to add a loading state when the issue details are being fetched to improve the user experience.
{issueId && issueDetails && searchInIssue && ( + <> + {!issueDetails ? ( + <div className="flex items-center justify-center p-4"> + <Loader className="w-5 h-5" /> + </div> + ) : ( <CommandPaletteIssueActions closePalette={closePalette} issueDetails={issueDetails} pages={pages} setPages={(newPages) => setPages(newPages)} setPlaceholder={(newPlaceholder) => setPlaceholder(newPlaceholder)} setSearchTerm={(newSearchTerm) => setSearchTerm(newSearchTerm)} /> + )} + </> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/command-palette/command-modal.tsx(1 hunks)
🔇 Additional comments (3)
web/core/components/command-palette/command-modal.tsx (3)
70-73: LGTM! Improved hook destructuring.The addition of
getIssueByIdfrom theuseIssueDetailhook aligns with the PR's objective to use data from the mobx store.
91-92: LGTM! Fixed stale data issue.The change to use
getIssueByIdwith the store data effectively resolves the mutation issue by ensuring the modal always displays the latest state.
84-89:Details
❓ Verification inconclusive
Verify SWR cache configuration.
While the SWR call is correctly implemented, consider adding a revalidation strategy to ensure data consistency when the modal is reopened.
🌐 Web query:
What are the recommended SWR revalidation strategies for modals?💡 Result:
When working with modals in applications using SWR, there are several recommended strategies to manage data revalidation effectively while maintaining optimal user experience:
Disabling Unwanted Automatic Revalidation
Modals often trigger focus events that cause unintended SWR revalidations. To prevent this:
Disable focus-based revalidation
useSWR(key, fetcher, { revalidateOnFocus: false })This stops SWR from refetching data when the modal opens/closes or regains focus[1][4].
Full automatic revalidation disable
For completely static modal content:useSWR(key, fetcher, { revalidateIfStale: false, revalidateOnFocus: false, revalidateOnReconnect: false })Or use the immutable helper:
import useSWRImmutable from 'swr/immutable' useSWRImmutable(key, fetcher)This ensures data loads once and never refreshes[1][4].
Manual Revalidation Control
When you need explicit refresh control:
const { data, mutate } = useSWR('/api/data', fetcher) // In modal close handler: const handleClose = () => { mutate() // Trigger manual validation }This pattern works well for forms/modals needing post-submission updates[7].
Deduplication Strategies
For modals that might open/close rapidly:
useSWR(key, fetcher, { dedupingInterval: 5000 // Extend dedupe window })Prevents duplicate requests during quick modal interactions while maintaining fresh data[5].
Optimistic Updates Pattern
For modals handling mutations:
const handleSubmit = async (formData) => { try { await mutate(key, updateAPI(formData), { optimisticData: formData, rollbackOnError: true }) } catch (error) { // Handle errors } }This provides instant UI feedback while safely handling server updates[3][7].
The key considerations are balancing stale data usage versus freshness requirements specific to your modal's purpose. For critical real-time data pair manual
mutate()calls with conservative automatic refresh settings[1][6].Citations:
- 1: https://swr.vercel.app/docs/revalidation
- 2: https://bejamas.com/hub/guides/choosing-the-best-rendering-strategy-for-your-next-js-app
- 3: https://soshace.com/2023/04/11/using-swr-for-efficient-data-fetching-in-next-js-applications/
- 4: vercel/swr#469
- 5: https://app.studyraid.com/en/read/11444/358636/request-deduplication-strategies
- 6: https://www.infoq.com/news/2020/11/ux-stale-while-revalidate/
- 7: https://swr.vercel.app/docs/mutation
SWR Revalidation Suggestion: Fine-Tune Your Modal’s Data Fetching Strategy
The current SWR hook is implemented correctly, but to avoid unintended revalidation when the modal reopens, consider adding a revalidation configuration. For example, you might add a third parameter to disable focus-based revalidation:
const { data: workItemDetailsSWR } = useSWR( workspaceSlug && workItem ? `ISSUE_DETAIL_${workspaceSlug}_${projectIdentifier}_${sequence_id}` : null, workspaceSlug && workItem ? () => fetchIssueWithIdentifier(workspaceSlug.toString(), projectIdentifier, sequence_id) : null, { revalidateOnFocus: false } // Prevents automatic revalidation when the modal regains focus );Alternatively, if your use case demands fresh data when the modal is reopened, you can control refresh behavior manually—calling the
mutate()function after the modal closes to trigger a revalidation only when needed.
* fix: command modal mutation * chore: minor update
Description
In this PR, I have fixed a mutation issue on command modal actions. This issue was observer when we were updating the work item detail from the command modal, the properties of the work item was updated but the command modal still showed stale data.
To Fix this issue, I have update the component to use data from
mobxstore instead of relying on the initialSWRcall.Type of Change
Screenshots and Media (if applicable)
Screen.Recording.2025-02-19.at.2.27.21.PM.mov
Screen.Recording.2025-02-19.at.2.26.44.PM.mov
Summary by CodeRabbit