Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: properly resolve sendMessage during memoization #2675

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

isekovanic
Copy link
Contributor

@isekovanic isekovanic commented Sep 18, 2024

🎯 Goal

We have an issue with memoization where if you do the following:

  • Override doSendMessageRequest on the Channel level
  • Make doSendMessageRequest depend on some local state that gets updated later

only the initial value of that state would be memoized.

Zendesk ticket: https://getstream.zendesk.com/agent/tickets/55303

🛠 Implementation details

The reason why we wrap sendMessage in a ref is due to the fact that simply adding it to the dependency array will likely cause a ton of rerenders of components everywhere. I realise it's a hacky way to solve this, but more conventional methods such as wrapping it inside a useCallback are a bit difficult since we would need to wrap all of its other dependencies in callbacks as well, causing a cascading effect. Ideally, this should be solved by getting rid of the useMemo entirely in the MessageInputContext, but for now this'll have to do.

🎨 UI Changes

iOS
Before After
Android
Before After

🧪 Testing

☑️ Checklist

  • I have signed the Stream CLA (required)
  • PR targets the develop branch
  • Documentation is updated
  • New code is tested in main example apps, including all possible scenarios
    • SampleApp iOS and Android
    • Expo iOS and Android

@Stream-SDK-Bot
Copy link
Contributor

Stream-SDK-Bot commented Sep 18, 2024

SDK Size

title develop branch diff status
js_bundle_size 442.099609375 KB 442 KB 0 B 🟢

@isekovanic isekovanic merged commit 87d85c6 into develop Sep 18, 2024
8 checks passed
@isekovanic isekovanic deleted the fix/send-message-memo-issue branch September 18, 2024 12:16
@github-actions github-actions bot mentioned this pull request Sep 18, 2024
6 tasks
@stream-ci-bot
Copy link
Contributor

🎉 This PR is included in version 5.38.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

khushal87 added a commit that referenced this pull request Sep 19, 2024
* fix: upgrade firebase versions to fix crash (#2646)

* fix: audio recording variety bugs (#2648)

* fix: variety of bugs in async audio feature

* fix: upload of empty messages and console.log cleanup

* fix: expo permission race conditions

* fix: create pure version of stopRecording() for usage on unmount

* fix: remove redundant export

* fix: actually amend failing test suites

* chore: remove constructors as they are not needed

* fix: pr remarks

* chore: write some simplistic functionality tests

* chore: return class instance rather than class itself for better usability

* chore: add more robust tests

* fix: linter errors

* fix: only display waveform whenever the mic is locked

* docs: add Deep Linking guide for the chat SDK (#2651)

* fix: change Block user action to Ban user action and UI cookbook for blocking users (#2649)

* fix: change Block user action to Ban user action

* fix: change Block user action to Ban user action

* fix: add back blockUser action and deprecate it

* docs: UI Cookbook for Blocking users

* chore(release): 5.36.1 [skip ci]

* chore(release): 1.28.1 [skip ci]

* fix: issue with loading app settings when the connectUser is not called on app (#2654)

* fix: issue with loading app settings when the connectUser is not called on app

* fix: add comments

* fix: add comments

* fix: unable to upload file due to special characters in the file name (#2656)

* fix: sdk size pr (#2657)

* [CI] Update SDK Size (#2653)

Co-authored-by: Khushal Agarwal <khushal.agarwal987@gmail.com>
Co-authored-by: Stream Bot <runner@fv-az1148-731.o3ts4yn1huletmfehy03vvfvxg.dx.internal.cloudapp.net>

* chore(release): 5.36.2 [skip ci]

* chore(release): 1.28.2 [skip ci]

* feat: add live location sharing cookbook (#2659)

* feat: add live location ui cookbook

* fix: prettier lint

* Update docusaurus/docs/reactnative/guides/live-location-sharing.mdx

Co-authored-by: Oliver Lazoroski <oliver.lazoroski@gmail.com>

* lint fixes

* fix lint issues

* fix locks

* move to ruby 3.1 and greater

---------

Co-authored-by: Oliver Lazoroski <oliver.lazoroski@gmail.com>

* chore(release): 5.37.0 [skip ci]

* chore(release): 1.29.0 [skip ci]

* fix: avoid prepending http before native supported url schemes (#2661)

* fix: avoid prepending http before native supported url schemes

* fix: move check to link parsing module

* fix: bad memoisation in window, screen dimension listener hooks (#2664)

* fix: bad memoisation in window, screen dimension listener hooks

* remove unused variable

* feat: add create chat client hook for easy usage (#2660)

* feat: add create chat client hook for easy usage

* docs: use useCreateChatClient hook for client creation

* fix: bump fastlane plugin version (#2665)

* fix: add theme properties for EmptyStateIndicator for message list (#2667)

* fix: add theme properties for EmptyStateIndicator for message list

* fix: update snapshots

* fix: apply card cover theme property order

* fix: pagination typescript errors and db synchronization bugs (#2669)

* fix: pagination typescript errors and db synchronization bugs

* chore: write test for db serialization issue

* fix: linter issues

* [CI] Bump max tolerance for sdk size analysis (#2674)

* chore: bump sample app version to v1.30.0

* fix: remove  nin and ne  operator usage in the SDK and the sample app (#2672)

* fix: remove  and  operator usage in the SDK and the sample app

* fix: remove console log

* fix: change console log to warn

* fix: add improvemnts

* fix: import of debouncefunc

* fix: import of debouncefunc

* fix: restructure queryMembers, queryUsers and ACItriggersettings

* fix: error bubbling for suggestions in auto complete input

* fix: request image access permissions for iOS only for native image picking (#2677)

* fix: properly resolve sendMessage during memoization (#2675)

* fix: properly resolve sendMessage during memoization

* fix: remedy change so that it does not cause performance issues

* chore: revert sendMessage in the dep array

* fix: deprecate messageReactions prop and use isMessageActionsVisible instead for messageActions (#2676)

* fix: deprecate messageReactions prop and use isMessageActionsVisible instead for messageActions

* docs: fix custom message actions

* fix: execution logic for showMessageOverlay

* chore(release): 5.38.0 [skip ci]

* chore(release): 1.29.1 [skip ci]

* fix: update yarn.lock for the project (#2681)

* fix: copy message action type for message actions (#2679)

* chore: update sdk size (#2678)

Co-authored-by: Khushal Agarwal <khushal.agarwal987@gmail.com>
Co-authored-by: Stream Bot <runner@fv-az1756-392.rxb2ubmju23uthz3oztawtjyeg.dx.internal.cloudapp.net>

* chore: resolve conflicts from develop

* fix: vale lint issues

* fix: new arch project config

---------

Co-authored-by: Ivan Sekovanikj <31964049+isekovanic@users.noreply.github.com>
Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>
Co-authored-by: Alexey Alter-Pesotskiy <alex@testableapple.com>
Co-authored-by: Stream SDK Bot <60655709+Stream-SDK-Bot@users.noreply.github.com>
Co-authored-by: Stream Bot <runner@fv-az1148-731.o3ts4yn1huletmfehy03vvfvxg.dx.internal.cloudapp.net>
Co-authored-by: Santhosh Vaiyapuri <3846977+santhoshvai@users.noreply.github.com>
Co-authored-by: Oliver Lazoroski <oliver.lazoroski@gmail.com>
Co-authored-by: Ivan Sekovanikj <ivan.sekovanikj@getstream.io>
Co-authored-by: Stream Bot <runner@fv-az1756-392.rxb2ubmju23uthz3oztawtjyeg.dx.internal.cloudapp.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants