Skip to content

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Nov 30, 2024

Fixes

Changes

To fix 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
  1. 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
  1. 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.

  • 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)

@AhyoungRyu AhyoungRyu self-assigned this Nov 30, 2024
@netlify
Copy link

netlify bot commented Nov 30, 2024

Deploy Preview for sendbird-uikit-react ready!

Name Link
🔨 Latest commit 072e57e
🔍 Latest deploy log https://app.netlify.com/sites/sendbird-uikit-react/deploys/674d07933625d6000895ed2c
😎 Deploy Preview https://deploy-preview-1263--sendbird-uikit-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines -33 to +37
const [isScrollBottomReached, setIsScrollBottomReached] = useState(true);
const {
state: { isScrollBottomReached },
actions: { setIsScrollBottomReached },
} = useGroupChannel();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@HoonBaek HoonBaek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@AhyoungRyu AhyoungRyu merged commit e4fb6a6 into feat/state-mgmt-migration-1 Dec 2, 2024
9 of 10 checks passed
@AhyoungRyu AhyoungRyu deleted the fix/CLNP-5918/CLNP-5917 branch December 2, 2024 01:07
AhyoungRyu added a commit that referenced this pull request Dec 2, 2024
…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
HoonBaek pushed a commit that referenced this pull request Dec 3, 2024
…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
AhyoungRyu added a commit that referenced this pull request Dec 11, 2024
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants