fix: Re-render components when use_mount dirties state#27
Merged
Conversation
Mount effects that mutate component state via use_mount were not triggering a re-render. Two issues combined to cause this: 1. fire_mount did not set needs_refresh after running mount handlers that dirtied state, so refresh_dirty_views never ran for them. 2. rebuild() unconditionally cleared needs_refresh *after* reconcile_children(), wiping any flag set by fire_mount during the reconciliation pass. Fix: clear needs_refresh before reconciliation so mount effects can set it back, and check the dirty flag after each mount handler fires. Adds a reproduction example (mount_rerender) that exercises the bug.
Greptile SummaryFixes a bug where Confidence Score: 5/5Safe to merge — targeted two-line ordering fix with no side effects on existing behavior. Both changes are minimal and correct. Moving the flag clear before reconciliation is safe, and the dirty check after mount handlers is additive. No regressions expected. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "cargo format" | Re-trigger Greptile |
This file contains hidden or 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
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.
Summary
use_mountwere not triggering a re-render, causing the component to display stale default state until an unrelated event forced a rebuild.rebuild()unconditionally clearedneeds_refreshafterreconcile_children(), wiping the flag thatfire_mountset during reconciliation. Additionally,fire_mountitself wasn't settingneeds_refreshat all.needs_refreshbefore reconciliation so mount effects can set it back, and check the dirty flag after each mount handler fires.Reproduction
cargo run --example mount_rerender— a component sets its label inuse_mount. Before this fix, the label shows[mount has not fired yet]until an external event triggers a re-render. After this fix, it correctly showsMounted with: hello from mountimmediately.Test plan
mount_rerenderexample demonstrates the fix visuallySessionContinuecomponent (the original trigger for this bug)