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

Ensure that widgets receive LifeCycle::WidgetAdded as first "event". #1127

Closed
luleyleo opened this issue Aug 12, 2020 · 1 comment · Fixed by #1259
Closed

Ensure that widgets receive LifeCycle::WidgetAdded as first "event". #1127

luleyleo opened this issue Aug 12, 2020 · 1 comment · Fixed by #1259
Labels
discussion needs feedback and ideas

Comments

@luleyleo
Copy link
Collaborator

We currently have the rule, that widgets can rely on LifeCycle::WidgetAdded being the first "event" (/update/layout/…) a widget receives. Yet we take no measures to ensure this is the case, and I would like to change that.

My first approach to this was preventing calls to update/`event/etc. by doing a "debug panic", i.e. only panic in debug mode, but log a message when running in release mode.

After implementing that I realized that it might be possible to just initialize any widget during those events if it has not been initialized yet, and it turned out that this would be possible as well.

But now I'm unsure which one would be the better solution. The first one allows easy pinpointing of where you did something wrong, while the second basically ignores it and just fixes your mess. The letter however would not fix the whole problem, because if you forgot to call children_changed (which is the main cause for a missing WidgetAdded), you also end up with a missing request_layout, which can not be solved lazily.

I think option one (panicking) is better, because it requires you to take proper care of it. The latter is mostly nice for prototyping I guess, but unless it makes children_changed completely redundant it would make things more difficult to debug instead.

@luleyleo luleyleo added the discussion needs feedback and ideas label Aug 12, 2020
@cmyr
Copy link
Member

cmyr commented Aug 26, 2020

My feeling here is that the assertion approach is better; it forces the programmer to be aware of their responsibilities, and also exposes situations where something unexpected is happening. In my experience solutions like the second end up papering over bugs, but don't help in the longer-term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needs feedback and ideas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants