Repository navigation
Restoration Framework - #60375
Restoration Framework#60375
Conversation
fluttergithubbot
added
framework
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
This is super cool! Was just reading through to see how you were doing it and found some minor typos.
dc69764 to
b01bec3
Compare
|
Tests for newly introduced concepts in the service layer ( |
ac736ba to
99efc80
Compare
99efc80 to
62016d2
Compare
fluttergithubbot
added
the
a: tests
|
This PR is now complete and ready for review, PTAL @gspencergoog and @Hixie. |
There was a problem hiding this comment.
nit: document that the object has to be cast-able to a Map<dynamic, dynamic>, otherwise will throw?
There was a problem hiding this comment.
If this is any kind of map this actually will not throw until you attempt to access something. The example below does not throw unless you uncomment the print.
void main() {
var intMap = <int, int>{2: 3};
var stringMap = intMap.cast<String, String>();
// print(stringMap.keys.first);
}
This also means the type checks and casts are incurred on every access, instead of once at the beginning. If you wanted something that threw eagerly, you could use Map.from instead
There was a problem hiding this comment.
Ahhh. Should we really be exposing this method then?
There was a problem hiding this comment.
Looking at this again, I don't need this method at all. I am removing it.
There was a problem hiding this comment.
Hashing the runtime type seems strange - why are we doing that? Can you document it here?
There was a problem hiding this comment.
Removed that, I think this was left-over from copying this from elsewhere.
There was a problem hiding this comment.
Shouldn't this be in an assert in the ctor body?
There was a problem hiding this comment.
I really want people to provide this, otherwise they will get bad error messages in case of duplicated IDs.
There was a problem hiding this comment.
nit: This would be more efficient as:
if (_parent != null) {
parent._dropChild(this);
parent_ = null;
}right?
There was a problem hiding this comment.
Why have this method and dispose?
There was a problem hiding this comment.
(IOW why not just fold it into dispose)
There was a problem hiding this comment.
Dispose and decommission are different. Decommission is a notification that this entire bucket subtree is getting replaced with a new bucket hierarchy. All the children are informed about this so the owner of them can dispose them and claim a new bucket.
Dispose simply removes the information of this bucket from the hierarchy. The child bucket continue to have a live of their own and the owners of those child buckets may get them adopted by other parents.
Decommission = notification down the tree to stop using any bucket in this hierarchy.
dispose = delete the data from this bucket, child buckets can continue to live if they want.
6956d31 to
f2e1d5f
Compare
|
Whoohoo! 🥳🥳🥳 |
* state restoration * added example * typos and analyzer * whitespace * more typos * remove unnecessary import * whitespace * fix sample code * tests for restorationmanager and restorationid * ++ * typo * tests for bucket, part1 * rename tests * more tests * finished tests for service layer * remove wrong todo * ++ * review comments * tests for Unmanaged and regular scope * RootRestorationScope tests * typo * whitespace * testing framework * tests for properties * last set of tests * analyzer * typo * dan review * whitespace * ++ * refactor finalizers * ++ * ++ * dispose guard * ++ * ++ * dan review * add manager assert * ++ * analyzer * greg review * fix typo * Ian & John review * ian review * RestorationID -> String * revert comment * Make primitives non-nullable in prep for NNBD
* state restoration * added example * typos and analyzer * whitespace * more typos * remove unnecessary import * whitespace * fix sample code * tests for restorationmanager and restorationid * ++ * typo * tests for bucket, part1 * rename tests * more tests * finished tests for service layer * remove wrong todo * ++ * review comments * tests for Unmanaged and regular scope * RootRestorationScope tests * typo * whitespace * testing framework * tests for properties * last set of tests * analyzer * typo * dan review * whitespace * ++ * refactor finalizers * ++ * ++ * dispose guard * ++ * ++ * dan review * add manager assert * ++ * analyzer * greg review * fix typo * Ian & John review * ian review * RestorationID -> String * revert comment * Make primitives non-nullable in prep for NNBD
10 participants
Marked as WIP because I am still working on the tests (I decided to refactor the original testing framework for state restoration a little more to make it easier to use).
Description
Implements https://flutter.dev/go/state-restoration-design.
This PR introduces the basic framework for state restoration in Flutter as outlined in the document above.
In the service layer, the following concepts are introduced:
RestorationManager, which synchronizes the restoration data between engine and framework.RestorationBucket, which stores the restoration data in a tree.RestorationId, which identifies a piece of restoration data uniquely within aRestorationBucket.In the widget layer, the following concepts are introduced. This is what most developers will presumably work with to make their apps restorable.
RestorationMixin, which is mixed into aStateobject to make certain values in the state restorable.RestorableProperty, which holds the data within aStateobject that need to be restorable.RestorablePropertyfor storing specific data types, e.g.RestorableInt,RestorableString, etc.RestorationScopewidgets to create new name spaces forRestorationIds in the widget tree.Related Issues
#6827
Tests
I added the following tests:
RestorationManager,RestorationBucket,RestorationIdand friends.Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.