-
Notifications
You must be signed in to change notification settings - Fork 28.9k
Build routes even less #58202
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
Build routes even less #58202
Conversation
cc @chunhtai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the optimization!
final List<String> log = <String>[]; | ||
Route<dynamic> nextRoute = PageRouteBuilder<int>( | ||
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) { | ||
log.add('building page 1 - ${ModalRoute.of(context).canPop}'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we care about can pop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It used to change value (that's why the pages rebuilt, basically). This is mostly making sure that it isn't rebuilding even if it's changing value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I understand, the doc string of canPop does mention it should rebuild when the value changes
/// When this changes, the route will rebuild, and any widgets that used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I should update those docs. Good catch!
Docs updated, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit deec88b (PR flutter#59415. This was originally landed via PR flutter#58202.
No description provided.