[CLNP-4657] Provider migration - P0 items #1269
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for sendbird-uikit-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
HoonBaek
force-pushed
the
feat/state-mgmt-migration-1
branch
from
b039a29 to
ca70cca
Compare
## Overview This PR refactors the MessageSearch state management system, introducing a custom store solution and several hooks for improved performance, maintainability, and type safety. ## New Files - `src/utils/storeManager.ts`: Implements a custom store creation utility - `src/contexts/_MessageSearchContext.tsx`: Provides the MessageSearch context and store \w new state mgmt logic. It's just temporal name. - `src/hooks/useStore.ts`: A generic hook for accessing and updating store state - `src/hooks/useMessageSearchStore.ts`: A specialized hook for MessageSearch state management - `src/components/MessageSearchManager.tsx`: Manages MessageSearch state and side effects - `src/hooks/useMessageSearchActions.ts`: Manages action handlers ## Updated Hooks - `useSetChannel`: Now uses `useMessageSearchStore` directly - `useSearchStringEffect`: Refactored to work with the new store - `useGetSearchMessages`: Updated to utilize the new state management system - `useScrollCallback`: Adapted to work with the custom store ## Key Changes 1. Introduced a custom store solution to replace the previous reducer-based state management. 2. Implemented `useStore` hook for type-safe and efficient state access and updates. 3. Created `MessageSearchManager` to centralize state management logic. 4. Refactored existing hooks to work with the new store system. 5. Improved type safety throughout the MessageSearch module.
[CLNP-5044](https://sendbird.atlassian.net/browse/CLNP-5044) ChannelSettingsProvider migration - Created a hook `useChannelSettings` for replacing `useChannelSettingsContext` - Created hooks `useSetChannel` and `useChannelHandler` to separate the logics from the ChannelSettingsProvider
https://sendbird.atlassian.net/browse/CLNP-5043 Two things are handled based on what I mentioned in https://sendbird.atlassian.net/wiki/spaces/UIKitreact/pages/2511765635/UIKit+React+new+State+Management+Method+Proposal#4.1-Unit-Test - [x] Added unit tests for `useMessageSearch` hook and new `MessageSearchProvider` - [x] Added integration tests for `MessageSearchUI` component So the MessageSearch module test coverage has been changed **from** File --------------------------------------------------| % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s <img width="814" alt="Screenshot 2024-10-08 at 2 36 55 PM" src="https://github.com/user-attachments/assets/c0fef6fe-0fc1-4f37-b74f-0486d70352a7"> **to** <img width="941" alt="after" src="https://github.com/user-attachments/assets/7fc19fb8-810c-4256-8230-3884d11e109a"> note that it used to be like 0%, but now the test coverage of the newly added files is almost 100%; green 🟩. ### Checklist Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If unsure, ask the members. This is a reminder of what we look for before merging your code. - [x] **All tests pass locally with my changes** - [x] **I have added tests that prove my fix is effective or that my feature works** - [ ] **Public components / utils / props are appropriately exported** - [ ] I have added necessary documentation (if appropriate)
This is a part of state management migration. This PR migrate 'GroupChannelListProvider' and related files to the new style. * Created `useGroupChannelList()` hook. It'll replace the previous `useGroupChannelContext()` hook.
fix: Prevent destroy error by adding `AbortController` in `useSetChannel` - Added `AbortController` to cancel async operations when the component unmounts. - Ensured that state updates are skipped if the operation is aborted. - Prevented potential memory leaks and the `'destroy is not a function'` error. - Updated `useEffect` cleanup to properly handle pending async calls. feat: Add tests for ChannelSettings migration
This PR contains the unit tests for the recent refactoring of `GroupChannelListProvider`. * add test about the new provider and its state management logic itself. * add the integration test with `GroupChannelListUI` component. --------- Co-authored-by: Irene Ryu <[email protected]>
…ern (#1246) Addresses https://sendbird.atlassian.net/browse/CLNP-5047 This PR migrates the GroupChannelProvider to use a new state management pattern. This change introduces a more predictable and maintainable way to manage channel state while maintaining backward compatibility. - Introduced new store-based state management - Separated concerns between state management and event handling - Added `useGroupChannel` hook for accessing state and actions - Optimized performance with proper memoization Old pattern: ```typescript const { someState, someHandler } = useGroupChannelContext(); ``` New pattern: ```typescript // For state and actions const { state, actions } = useGroupChannel(); // For handlers and props (backward compatibility) const { someHandler } = useGroupChannelContext(); ``` - More predictable state updates - Better separation of concerns - Enhanced performance through optimized renders - All existing functionality remains unchanged - Added tests for new hooks and state management - Verified backward compatibility - Tested with real-time updates and message handling - [x] useGroupChannelContext is kept for backward compatibility - [x] Unit & integration tests will be added for new hooks and state management Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If unsure, ask the members. This is a reminder of what we look for before merging your code. - [x] **All tests pass locally with my changes** - [x] **I have added tests that prove my fix is effective or that my feature works** - [ ] **Public components / utils / props are appropriately exported** - [ ] I have added necessary documentation (if appropriate)
#1250) ### Overview This PR migrate ThreadProvider and related files to the new state management pattern. ### Changelog * `ThreadProvider` is migrated, and `useThread()` hook is introduced. * Removed `ThreadDispatcher` usages in ThreadProvider; it is replaced with the new state-action pattern of `useThread()`. * `PubSub` of `config` still remains. It is out of scope of this PR. ### Remaining tasks * Add unit tests and integration tests. ### FurtherConcern * Handling hook * The previous `ThreadProvider` contained several custom hooks. Those hooks retrieved state and actions through `useThreadContext()` * Due to that, replacing `useThreadContext()` to new `useThread()` faced a problem. Those hooks �conatin `useThread()`, `useThread()` contains the hooks. So it makes cycle. * For now, I moved all functionality of the hooks to the `useThread()`, but it looks wrong. Any good way to handle this?
…annelProvider (#1263) Fixes - https://sendbird.atlassian.net/browse/CLNP-5917 - https://sendbird.atlassian.net/browse/CLNP-5918 ### Changes To fix [CLNP-5917](https://sendbird.atlassian.net/browse/CLNP-5917) introduced optimizations to prevent the "Maximum update depth exceeded" error that occurs during message searches: 1. Added useDeepCompareEffect hook: - Performs deep comparison of dependencies instead of reference equality - Particularly useful for handling message array updates efficiently - Inspired by [kentcdodds/use-deep-compare-effect](https://github.com/kentcdodds/use-deep-compare-effect) 2. Enhanced useStore with state change detection: - Added hasStateChanged helper to compare previous and next states - Prevents unnecessary updates when state values haven't actually changed - Optimizes performance by reducing redundant renders 3. Improved storeManager with nested update prevention: - Added protection against nested state updates - Prevents infinite update cycles Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If unsure, ask the members. This is a reminder of what we look for before merging your code. - [x] **All tests pass locally with my changes** - [ ] **I have added tests that prove my fix is effective or that my feature works** - [ ] **Public components / utils / props are appropriately exported** - [ ] I have added necessary documentation (if appropriate) [CLNP-5917]: https://sendbird.atlassian.net/browse/CLNP-5917?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
* Changelog * Fixed the broken unit tests of `GroupChannelListProvider` * Fixed the UI error when click `open in channel` in the other channel * Use `useDeepCompareEffect` in `ThreadProvider` and `GroupChannelListProvider`
) Used `useDeepCompareEffect` for the `updateState` of `SendbirdProvider` Co-authored-by: Irene Ryu <[email protected]>
- before <img width="1099" alt="Screenshot 2024-12-03 at 11 14 47 PM" src="https://github.com/user-attachments/assets/75716138-2ac6-46d1-8fe7-d3793d53c3be"> - after <img width="1107" alt="Screenshot 2024-12-03 at 11 13 43 PM" src="https://github.com/user-attachments/assets/182237d5-c322-4d59-8595-3abdaf15afa9"> GroupChannelProvider has some not covered lines but they're mostly from uikit-tools lib related logic. There could be a way to mock these lines too, but let me handle it separately.
…ider (#1272) Fixes - https://sendbird.atlassian.net/browse/CLNP-5966 - https://sendbird.atlassian.net/browse/CLNP-5967 - https://sendbird.atlassian.net/browse/CLNP-5969 - https://sendbird.atlassian.net/browse/CLNP-5971 - https://sendbird.atlassian.net/browse/CLNP-5973 ## When to use useDeepCompareEffect vs useEffect ### useDeepCompareEffect is useful when: 1. **Handling objects without guaranteed immutability** ```typescript const complexObject = { settings: { theme: { ... }, preferences: { ... } }, data: [ ... ] }; useDeepCompareEffect(() => { // When you want to detect actual value changes }, [complexObject]); ``` 2. **Working with data from external libraries or APIs** - When objects have new references but identical content 3. **Dealing with deeply nested objects where memoization is impractical** - When object structures are too complex for individual memoization ### useEffect is better when: 1. **Detecting changes in array items** ```typescript const items = [{id: 1, value: 'a'}, {id: 2, value: 'b'}]; // Better for detecting changes within array items useEffect(() => { // Detect changes in items array }, [items]); ``` 2. **Performance is critical** - Deep comparison is computationally expensive - Especially important for large arrays or frequently updating data ### Example of proper useDeepCompareEffect usage: ```typescript useDeepCompareEffect(() => { updateState({ ...configurations, ...scrollState, ...eventHandlers, }); }, [ configurations, scrollState, eventHandlers, ]); ``` This works well here because: - Dependencies are mostly objects - Updates are needed only when internal structure changes - Objects are already memoized, reducing deep comparison cost ### Key Takeaway: - Use useDeepCompareEffect when structural equality matters - Use useEffect for reference equality or primitive value changes - Consider the trade-off between performance and accuracy --------- Co-authored-by: junyoung.lim <[email protected]>
Fixes https://sendbird.atlassian.net/browse/CLNP-5981 and applied the same approach in #1272 ### Checklist Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If unsure, ask the members. This is a reminder of what we look for before merging your code. - [x] **All tests pass locally with my changes** - [ ] **I have added tests that prove my fix is effective or that my feature works** - [ ] **Public components / utils / props are appropriately exported** - [ ] I have added necessary documentation (if appropriate)
[CLNP-5737](https://sendbird.atlassian.net/browse/CLNP-5737) * Added tests for `Sendbird/index.tsx`, `Sendbird/utils.ts`, `SendbirdProvider.tsx`, `initialState.ts`, and `useSendbird.tsx` ### Before  ### After 
### Changelog This PR will add the test cases for the new `ThreadProvider` and its related hooks. #### Before <img width="1163" alt="스크린샷 2024-12-09 오전 11 29 53" src="https://github.com/user-attachments/assets/40ffc29e-0774-4ac7-973d-15af55bc88fd"> #### After <img width="1199" alt="스크린샷 2024-12-09 오전 11 28 39" src="https://github.com/user-attachments/assets/ea3366c0-2f11-4e1f-af6f-a423d006ab42"> --------- Co-authored-by: Irene Ryu <[email protected]>
…ok (#1278) #### Before <img width="705" alt="Screenshot 2024-12-06 at 6 23 24 PM" src="https://github.com/user-attachments/assets/8269506e-b347-4091-a3d7-254aabb063d9"> #### After <img width="703" alt="Screenshot 2024-12-06 at 6 36 19 PM" src="https://github.com/user-attachments/assets/4480241a-4bfd-4096-a13f-27f7c6ac76ba">
) Fixes https://sendbird.atlassian.net/browse/CLNP-6022 This PR addresses an issue where the scroll position was not correctly set to the bottom when switching from a channel with few messages to one with many messages. The problem was resolved by adding a delay to ensure the scroll reference is updated before attempting to scroll to the bottom. This change ensures that users always see the latest messages when switching channels.
AhyoungRyu
force-pushed
the
feat/state-mgmt-migration-1
branch
from
68e0cb8 to
21a625d
Compare
### Description Fixed the failures of migration tests added by #1279
### Changelog * Added tests for hooks in `src/hooks` #### before <img width="1186" alt="스크린샷 2024-12-10 오후 4 09 19" src="https://github.com/user-attachments/assets/b22b914a-1280-4d22-b4f1-3e735fdd409e"> #### after <img width="1099" alt="스크린샷 2024-12-10 오후 4 08 14" src="https://github.com/user-attachments/assets/3431656d-ff7a-4bfd-9df6-ee255d022dee">
…-rendering (#1288) Addresses https://sendbird.atlassian.net/browse/CLNP-6022?focusedCommentId=301263 ### Key changes Memoized action handlers(+ scroll related functions as well) in useGroupChannel to reduced unnecessary re-rendering.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses P0 items in https://sendbird.atlassian.net/browse/CLNP-4657
This PR is for merging a mega branch
feat/state-mgmt-migration-1intomain.