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

Crash if length of children changes #23

Open
milesegan opened this issue Oct 31, 2023 · 6 comments
Open

Crash if length of children changes #23

milesegan opened this issue Oct 31, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@milesegan
Copy link

milesegan commented Oct 31, 2023

Description
The LazyIndexedStack can crash if the length of children changes.

Steps To Reproduce

  1. Render the widget.
  2. Change the length of children and then change the index to point to one of the new children.
  3. The widget will crash trying to index past the end of the _activatedChildren list.

late final List<bool> _activatedChildren;

Maybe it would be safer to use a set like this:

  late final _activatedChildren = <int>{widget.index};

Thanks for the library by the way!

@milesegan milesegan added the bug Something isn't working label Oct 31, 2023
@marcossevilla
Copy link
Owner

hi @milesegan! I will look into this but I was curious what was the use case that generated this issue, like why did you change the length of the children and then pointed to one that wasn't on the list? This would help me understand better how the API should solve your issue without breaking changes 👍

@marcossevilla marcossevilla self-assigned this Oct 31, 2023
@milesegan
Copy link
Author

@marcossevilla In my case I have a responsive app that has a different number of items depending on whether it's on a small or a large screen. So when the user resizes the app from small to large the number of items in the stack increases. Maybe this is an unusual situation?

In any case I think you could make this change to the implementation without changing anything in the API of LazyIndexedStack.

@marcossevilla
Copy link
Owner

@milesegan sorry for the late reply, I can try reassigning the children on didUpdateWidget if they change but this would reset the stack active children. would that be the behavior you're expecting?

@milesegan
Copy link
Author

@marcossevilla Thanks for the reply. I've been using a patched version of the stack storing them in a set instead of a list like this:

  late final _activatedChildren = <int>{widget.index};

This has been working well for me for a while now.

@marcossevilla
Copy link
Owner

@milesegan alright, you feel that should be the way it's handled in the package? I don't really see advantages aside from your use case.

@milesegan
Copy link
Author

Well I think the main issue with the current implementation is that it's quite easy to crash the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants