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

Remove intermediate event sinks #733

Merged
merged 1 commit into from
Jul 14, 2023
Merged

Remove intermediate event sinks #733

merged 1 commit into from
Jul 14, 2023

Conversation

ZacSweers
Copy link
Collaborator

I can no longer reproduce the original issue. While the recomposition factor does come into play, this seems more or less acceptable that the recomposition can happen when the state updates. This simplifies our usage a fair bit, and I kind of wonder if the original bug was something else wrong with the compose code we'd written at the time. I think we should run ahead with this until/unless we see reproducible issues with it.

I can no longer reproduce the original issue. While the recomposition factor does come into play, this seems more or less acceptable that the recomposition can happen when the state updates. This simplifies our usage a fair bit, and I kind of wonder if the original bug was something else wrong with the compose code we'd written at the time. I think we should run ahead with this until/unless we see reproducible issues with it.
@ZacSweers ZacSweers requested a review from kierse July 13, 2023 19:09
@ZacSweers ZacSweers enabled auto-merge July 13, 2023 21:17
Copy link
Collaborator

@kierse kierse left a comment

Choose a reason for hiding this comment

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

I agree, recomposing when the state changes seems reasonable

@ZacSweers ZacSweers added this pull request to the merge queue Jul 14, 2023
Merged via the queue into main with commit 2ea6537 Jul 14, 2023
@ZacSweers ZacSweers deleted the z/sinkCleanups branch July 14, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants