[FD-434] 프론트 몇가지 개선 #281
[FD-434] 프론트 몇가지 개선 #281
Conversation
…into FD-434--fe-issue
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update enhances the front-end application’s push notification and state management functionalities. A new asynchronous function in the service worker now sends messages to all active window clients concurrently with notifications. Several API hooks have been updated to include query invalidation and a new service worker message hook has been introduced. Import statements in components were streamlined by removing unused hooks, and components managing notifications and team data now incorporate updated state handling and animations. Changes
Sequence Diagram(s)sequenceDiagram
participant SW as Service Worker
participant Client as Window Client
participant Main as MainPage Component
SW->>SW: Push event received
par Concurrent Operations
SW->>SW: showNotification()
SW->>Client: sendMessageToClients(notificationType)
end
Client->>Client: useServiceWorkerMessage listens for messages
Client->>Main: Trigger cache invalidation (invalidateNotification/invalidateMainCard)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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: 1
🧹 Nitpick comments (4)
front-end/public/service-worker.js (1)
98-107: Well-structured client messaging function.The
sendMessageToClientsfunction is properly implemented to communicate with all active window clients. This enables the service worker to notify the client about push notifications, which can be used to invalidate caches as mentioned in the PR objectives.However, consider adding error handling for potential failures in client messaging:
async function sendMessageToClients(type) { - const clients = await self.clients.matchAll({ - type: 'window', - includeUncontrolled: true, - }); - const data = { type }; - for (const client of clients) { - client.postMessage(data); - } + try { + const clients = await self.clients.matchAll({ + type: 'window', + includeUncontrolled: true, + }); + const data = { type }; + for (const client of clients) { + client.postMessage(data); + } + } catch (error) { + console.error('Error sending message to clients:', error); + } }front-end/src/pages/teamspace/TeamSpaceMake.jsx (1)
60-60: Use functional update pattern for state updates.When updating state based on previous state, use the functional update pattern to avoid potential race conditions.
-setTeams([...teams, teamData]); +setTeams(prevTeams => [...prevTeams, teamData]);front-end/src/api/usePushNoti.js (1)
102-113: Add type checking for message handling.The current implementation doesn't check if the received message data has a
typeproperty before trying to use it.const handleServiceWorkerMessage = (event) => { const data = event.data; + if (!data || typeof data !== 'object' || !('type' in data)) { + console.warn('Received invalid message format from service worker:', data); + return; + } if (allMessageType) { onMessage(data); } else if (messageTypes.includes(data.type)) { onMessage(data); } };front-end/src/api/useMainPage.js (1)
26-29: Consider adding error handling for the query.While the implementation is correct, consider destructuring and returning isLoading and isError states as well to provide consumers with more information about the query status.
- const { data } = useQuery({ + const { data, isLoading, isError } = useQuery({ queryKey: ['mainCard', teamId], queryFn: () => api.get({ url: `/api/team/${teamId}/schedule` }), enabled: !!teamId, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
front-end/public/service-worker.js(2 hunks)front-end/src/api/useMainPage.js(4 hunks)front-end/src/api/usePushNoti.js(3 hunks)front-end/src/components/Accordion.jsx(1 hunks)front-end/src/pages/main/MainPage.jsx(7 hunks)front-end/src/pages/teamspace/TeamSpaceMake.jsx(2 hunks)
🔇 Additional comments (22)
front-end/public/service-worker.js (1)
74-78: Good implementation of concurrent notification processing.The use of
Promise.allSettledensures that both the client messaging and notification display happen concurrently without one blocking the other. This is a robust approach for handling multiple asynchronous operations.front-end/src/pages/teamspace/TeamSpaceMake.jsx (1)
33-33: LGTM - Added additional team state management.Properly expanded the destructuring assignment to include
setTeamsandteamsfrom the useTeam hook.front-end/src/components/Accordion.jsx (1)
1-5: Import cleanup and update.Good removal of unused React hooks and update to the named import of
usePushNotito match its export declaration.front-end/src/api/usePushNoti.js (1)
37-37: Export type changed from default to named.Changed the export of
usePushNotifrom default to named export for better import/export consistency.front-end/src/api/useMainPage.js (9)
1-1: Good use of useCallback.Adding the useCallback import supports the optimization of function references through memoization, which is utilized effectively in the new invalidation functions.
20-20: JSDoc update properly reflects new return values.The updated JSDoc comment accurately documents the new return structure of the useMainCard hook, making it clear for other developers.
23-25: Proper hook placement.Correctly initialized the queryClient at the top level of the component, following React's rules of hooks.
32-37: Good implementation of memoized invalidation function.The use of useCallback with proper dependencies ensures the function reference stability across renders, which is important for performance optimization.
39-42: Return structure updated correctly.The return object now properly includes both data and the invalidation function, matching the updated JSDoc.
60-60: JSDoc update properly reflects new return values.The updated JSDoc comment accurately documents the new return structure of the useNotification hook.
72-75: Nice implementation of callback with inline function.Good use of the useCallback pattern with an inline function for brevity while maintaining the same dependency array pattern.
80-80: Great simplification of onSuccess callback.Replaced direct query invalidation with the memoized function, making the code more maintainable and consistent.
88-88: Return structure updated correctly.Added the invalidateNotification function to the return object, making it available to consumers.
front-end/src/pages/main/MainPage.jsx (9)
33-33: Good import update for service worker message handling.Updated import to include the new useServiceWorkerMessage hook, which will be used to handle service worker communication.
59-63: Updated destructuring for useNotification hook.Properly destructured to include the new invalidateNotification function.
78-88: Good implementation of service worker message handlers.Added useServiceWorkerMessage hooks to invalidate cache when specific push messages are received, improving the real-time update capability of the application.
The comment explains the purpose clearly and the implementation correctly maps specific message types to the appropriate invalidation functions.
169-170: Well-defined custom easing function.Added a customEasing array with carefully chosen values to cover layout shifting time, improving animation smoothness.
178-178: Animation transition improved with custom easing.Applied the custom easing function to the main container animation, which should result in smoother transitions.
199-199: Enhanced notification banner entrance animation.Doubled the initial y-offset from -10 to -20, which will make the entrance animation more noticeable and provide a smoother appearance.
207-209: Improved opacity transition with custom easing.Updated the opacity transition to use the custom easing function instead of the default, providing a more consistent animation feel throughout the application.
227-227: Consistent animation updates across components.Applied the same custom easing to all motion components in the page, ensuring a consistent animation experience throughout the UI.
Also applies to: 245-245
56-57:Details
✅ Verification successful
Updated hook destructuring to include invalidation capability.
Now consuming the invalidateMainCard function from the useMainCard hook, but you're no longer destructuring isPending. Verify if this was intentional.
🏁 Script executed:
#!/bin/bash # Check if isPending is used elsewhere in the file rg "isMainCardPending" front-end/src/pages/main/MainPage.jsxLength of output: 60
🏁 Script executed:
#!/bin/bash # Check if 'isPending' is referenced anywhere in MainPage.jsx rg "isPending" front-end/src/pages/main/MainPage.jsxLength of output: 52
Attention: Invalidation Update Confirmed
After verifying the usage in the file, it appears that the previously destructuredisPendingproperty is not referenced anywhere infront-end/src/pages/main/MainPage.jsx. This suggests that its removal was intentional and does not affect any functionality. Please confirm that you do not plan to utilize a pending indicator in this component. If you intend to handle any asynchronous state indicators later, consider reintroducing it accordingly.
- Verified that no instance of
isPendingis used.- Removal seems safe as long as no pending state logic is required.
| /** | ||
| * Service Worker로부터 메시지를 받는 Hook | ||
| * @param {function} onMessage - 메시지를 받았을 때 실행할 함수 | ||
| * @param {string[]} messageTypes - 받을 메시지 타입 | ||
| */ | ||
| export const useServiceWorkerMessage = (onMessage, messageTypes) => { | ||
| const allMessageType = useMemo(() => !messageTypes, [!!messageTypes]); | ||
|
|
||
| useEffect(() => { | ||
| const handleServiceWorkerMessage = (event) => { | ||
| const data = event.data; | ||
| if (allMessageType) { | ||
| onMessage(data); | ||
| } else if (messageTypes.includes(data.type)) { | ||
| onMessage(data); | ||
| } | ||
| }; | ||
|
|
||
| if ('serviceWorker' in navigator) { | ||
| navigator.serviceWorker.addEventListener( | ||
| 'message', | ||
| handleServiceWorkerMessage, | ||
| ); | ||
| } | ||
|
|
||
| return () => { | ||
| if ('serviceWorker' in navigator) { | ||
| navigator.serviceWorker.removeEventListener( | ||
| 'message', | ||
| handleServiceWorkerMessage, | ||
| ); | ||
| } | ||
| }; | ||
| }, [messageTypes, allMessageType, onMessage]); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Well-implemented service worker message hook.
The useServiceWorkerMessage hook provides a clean interface for components to listen for and respond to service worker messages, which aligns with the PR objectives.
However, there's an issue with the allMessageType memoization:
- const allMessageType = useMemo(() => !messageTypes, [!!messageTypes]);
+ const allMessageType = useMemo(
+ () => !messageTypes || messageTypes.length === 0,
+ [messageTypes]
+ );The current implementation will not correctly detect an empty array, as !!messageTypes will be true for an empty array. This could lead to incorrect message filtering.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Service Worker로부터 메시지를 받는 Hook | |
| * @param {function} onMessage - 메시지를 받았을 때 실행할 함수 | |
| * @param {string[]} messageTypes - 받을 메시지 타입 | |
| */ | |
| export const useServiceWorkerMessage = (onMessage, messageTypes) => { | |
| const allMessageType = useMemo(() => !messageTypes, [!!messageTypes]); | |
| useEffect(() => { | |
| const handleServiceWorkerMessage = (event) => { | |
| const data = event.data; | |
| if (allMessageType) { | |
| onMessage(data); | |
| } else if (messageTypes.includes(data.type)) { | |
| onMessage(data); | |
| } | |
| }; | |
| if ('serviceWorker' in navigator) { | |
| navigator.serviceWorker.addEventListener( | |
| 'message', | |
| handleServiceWorkerMessage, | |
| ); | |
| } | |
| return () => { | |
| if ('serviceWorker' in navigator) { | |
| navigator.serviceWorker.removeEventListener( | |
| 'message', | |
| handleServiceWorkerMessage, | |
| ); | |
| } | |
| }; | |
| }, [messageTypes, allMessageType, onMessage]); | |
| }; | |
| /** | |
| * Service Worker로부터 메시지를 받는 Hook | |
| * @param {function} onMessage - 메시지를 받았을 때 실행할 함수 | |
| * @param {string[]} messageTypes - 받을 메시지 타입 | |
| */ | |
| export const useServiceWorkerMessage = (onMessage, messageTypes) => { | |
| const allMessageType = useMemo( | |
| () => !messageTypes || messageTypes.length === 0, | |
| [messageTypes] | |
| ); | |
| useEffect(() => { | |
| const handleServiceWorkerMessage = (event) => { | |
| const data = event.data; | |
| if (allMessageType) { | |
| onMessage(data); | |
| } else if (messageTypes.includes(data.type)) { | |
| onMessage(data); | |
| } | |
| }; | |
| if ('serviceWorker' in navigator) { | |
| navigator.serviceWorker.addEventListener( | |
| 'message', | |
| handleServiceWorkerMessage, | |
| ); | |
| } | |
| return () => { | |
| if ('serviceWorker' in navigator) { | |
| navigator.serviceWorker.removeEventListener( | |
| 'message', | |
| handleServiceWorkerMessage, | |
| ); | |
| } | |
| }; | |
| }, [messageTypes, allMessageType, onMessage]); | |
| }; |
| 'scheduleCreate', | ||
| 'regularFeedbackRequest', | ||
| ]); | ||
|
|
There was a problem hiding this comment.
invalidate 함수들 별개로 만든거 보기좋군요
Summary by CodeRabbit