Skip to content

Commit a5268cd

Browse files
authored
story: Avoid retaining PopoverStory from popover child (#2378)
## Summary While reading the story examples to learn how to use, I noticed a potential memory leak in the popover story. `Form` and `DropdownListDelegate` were holding a strong `Entity<PopoverStory>` reference back to their parent. Since these child entities are owned by `PopoverStory`, keeping a strong parent reference can create a retain cycle. This PR changes those parent references to `WeakEntity<PopoverStory>` and safely ignores updates when the parent has already been dropped. ## Changes - Replace strong `Entity<PopoverStory>` parent references with `WeakEntity<PopoverStory>`. - Use `cx.weak_entity()` when passing the parent into `Form` and `DropdownListDelegate`. - Handle `WeakEntity::update` results without assuming the parent still exists. ## Notes Thanks for providing such a great component set. I found this while studying the story examples and wanted to fix a small lifecycle issue that could otherwise be easy to miss.
1 parent ccc1e76 commit a5268cd

1 file changed

Lines changed: 17 additions & 15 deletions

File tree

crates/story/src/stories/popover_story.rs

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use gpui::{
22
Action, Anchor, App, AppContext, Context, DismissEvent, Entity, EventEmitter, FocusHandle,
33
Focusable, Half, InteractiveElement, IntoElement, KeyBinding, MouseButton, ParentElement as _,
4-
Render, Styled as _, Window, actions, div, px,
4+
Render, Styled as _, WeakEntity, Window, actions, div, px,
55
};
66
use gpui_component::{
77
ActiveTheme, StyledExt, WindowExt,
@@ -45,14 +45,14 @@ pub fn init(cx: &mut App) {
4545
}
4646

4747
struct Form {
48-
parent: Entity<PopoverStory>,
48+
parent: WeakEntity<PopoverStory>,
4949
input1: Entity<InputState>,
5050
}
5151

5252
impl Form {
53-
fn new(parent: Entity<PopoverStory>, window: &mut Window, cx: &mut App) -> Entity<Self> {
53+
fn new(parent: WeakEntity<PopoverStory>, window: &mut Window, cx: &mut App) -> Entity<Self> {
5454
cx.new(|cx| Self {
55-
parent,
55+
parent: parent,
5656
input1: cx.new(|cx| InputState::new(window, cx)),
5757
})
5858
}
@@ -65,8 +65,9 @@ impl Focusable for Form {
6565
}
6666

6767
struct DropdownListDelegate {
68-
parent: Entity<PopoverStory>,
68+
parent: WeakEntity<PopoverStory>,
6969
}
70+
7071
impl ListDelegate for DropdownListDelegate {
7172
type Item = ListItem;
7273

@@ -92,17 +93,17 @@ impl ListDelegate for DropdownListDelegate {
9293
}
9394

9495
fn confirm(&mut self, _: bool, _: &mut Window, cx: &mut Context<ListState<Self>>) {
95-
self.parent.update(cx, |this, cx| {
96+
let _ = self.parent.update(cx, |this, cx| {
9697
this.list_popover_open = false;
9798
cx.notify();
98-
})
99+
});
99100
}
100101

101102
fn cancel(&mut self, _: &mut Window, cx: &mut Context<ListState<Self>>) {
102-
self.parent.update(cx, |this, cx| {
103+
let _ = self.parent.update(cx, |this, cx| {
103104
this.list_popover_open = false;
104105
cx.notify();
105-
})
106+
});
106107
}
107108
}
108109

@@ -123,10 +124,10 @@ impl Render for Form {
123124
.label("Submit")
124125
.primary()
125126
.on_click(cx.listener(move |_, _, _, cx| {
126-
parent.update(cx, |this, cx| {
127+
let _ = parent.update(cx, |this, cx| {
127128
this.form_popover_open = false;
128129
cx.notify();
129-
})
130+
});
130131
})),
131132
)
132133
}
@@ -162,10 +163,11 @@ impl PopoverStory {
162163
}
163164

164165
fn new(window: &mut Window, cx: &mut Context<Self>) -> Self {
165-
let form = Form::new(cx.entity(), window, cx);
166-
let parent = cx.entity();
167-
let list = cx
168-
.new(|cx| ListState::new(DropdownListDelegate { parent }, window, cx).searchable(true));
166+
let form = Form::new(cx.weak_entity(), window, cx);
167+
let parent = cx.weak_entity();
168+
let list = cx.new(|cx| {
169+
ListState::new(DropdownListDelegate { parent: parent }, window, cx).searchable(true)
170+
});
169171

170172
cx.focus_self(window);
171173

0 commit comments

Comments
 (0)