Add support for using RestorableProperty with hook

[blaugold]

This PR adds support for managing RestorablePropertys with the help of a hook and HookRestorationScope.

[rrousselGit]

We shouldn't need such widget. Relying on an InheritedWidget to work around the RestorationMixin could be a significant source of problems.

I would expect people to easily forget to wrap a custom HookWidget using useRestorableProperty into a HookRestorationScope – and in fact, they shouldn't have to.

useRestorableProperty should be able to handle everything by itself

[blaugold]

I agree, it's not optimal. The reason I implemented it this way is that the implementation of RestorableProperty is tied to RestorationMixin. I have not looked into if it would be possible to use RestorableProperty without RestorationMixin but that doesn't seem like a good idea. If users can't use RestorableProperty, they also can't use all the subclasses which would mean quite a large part of the user facing restoration API would need to be replaced somehow. I'll take a second look at how to get around HookRestorationScope.

[rrousselGit]

The problem is, we can't reasonably expose the RestorationMixin through an InheritedWidget

That could cause major issues, as the mixin could suddenly be associated with multiple HookWidget

[rrousselGit]

Conditionally calling hooks is anti-pattern. The listen flag probably shouldn't be there at all

[blaugold]

I assumed most users will want to listen to the property changes but thinking about it I'm not so sure. I'll remove the option and document that properties need to be listened to explicitly for example with useListenable.

[rrousselGit]

Sorry, I meant "always listen" instead, rather than optionally listen.

If we want to support optional listening, this should probably be in a different hook rather than as a flag

[rrousselGit]

I think there are a few paths we can explore for the RestorationMixin issue.

In the end, the only thing we care about here is passing the restorationId. Then everything else can be managed by hooks or HookElement.

So we have a few options:

• A getter on the wiget:

class Example extends HookWidget {
  @override
  String get restorationId => 'foo';
}

• A custom hook

class Example extends HookWidget {
  @override
  Widget build(context) {
    useRestoration('foo');
    final counter = useRestorableInt(0);
  }
}

I also believe that we shouldn't have a single useRestorableProperty but rather multiple variants of useState that are restorable, so:

final count = useRestorableInt(0, 'count');
final str = useRestorableString('', 'str');
...

[TimDepping]

I think there are a few paths we can explore for the RestorationMixin issue.

In the end, the only thing we care about here is passing the restorationId. Then everything else can be managed by hooks or HookElement.

So we have a few options:

• A getter on the wiget:

class Example extends HookWidget {
  @override
  String get restorationId => 'foo';
}

• A custom hook

class Example extends HookWidget {
  @override
  Widget build(context) {
    useRestoration('foo');
    final counter = useRestorableInt(0);
  }
}

I also believe that we shouldn't have a single useRestorableProperty but rather multiple variants of useState that are restorable, so:

final count = useRestorableInt(0, 'count');
final str = useRestorableString('', 'str');
...

Could you share your thoughts on how to face the RestorationMixin issue?

[blaugold]

I have incorporated the comments from @rrousselGit in this API proposal:

Enabling restoration for hook widget

RestorationBucket? useRestoration(String? restorationId);

This hook enables restoration for a hook widget. It can be used only once in a hook widget. It must have been used before other restoration hooks are used.

It takes the restorationId which should be used for the widget. null is accepted to signal that restoration is disabled for the widget (similar to how RestorationMixin works).

It returns the RestorationBucket associated with the widget if one is available. This is useful to expose a widget's bucket to its children, through UnmanagedRestorationScope.

Using a RestorableProperty

T useRestorableProperty<T extends RestorableProperty<dynamic>>(
  ValueGetter<T> createProperty, 
  String restorationId,
);

This hook takes a factory, creates the property during initialization and memorizes it. It also takes the restorationId for the property and uses this id to register the property for restoration.

The value of the hook is the memorized property. The hook listens to the property and rebuilds when it changes.

Hooks for existing RestorableProperty implementations

For existing implementations of RestorableProperty hooks similar to this one are provided:

RestorableInt useRestorableInt(int defaultValue, String restorationId) {
  return useRestorableProperty(
    () => RestorableInt(defaultValue),
    restorationId,
  );
}

Implementation

To facilitate the use of RestorableProperty, HookElement manages an internal widget which serves as the hosts for a RestorationMixin.

[rrousselGit]

To facilitate the use of RestorableProperty, HookElement manages an internal widget which serves as the hosts for a RestorationMixin.

What do you mean by this?

[blaugold]

We need a RestorationMixin somewhere to register RestorablePropertys with. The idea is to create a StatefulWidget, whose State mixes in RestorationMixin and just passes though a child. When restoration is enabled for a widget, HookElement inserts this widget above the widget built by it. This widget provides APIs for setting the restorationId, accessing theRestorationBucket it is managing and registering properties for restoration.

[rrousselGit]

We don't need the RestorationMixin, but rather its implementation

It's a story similar to SingleTickerProviderStateMixin. flutter_hooks does not use the official mixin, but rather copy-pasted its implementation and converted it in a hook.

We could do the same thing here, and have useRestorationProperty host the implementation

[blaugold]

If we want to avoid RestorationMixin we cannot use RestorableProperty because its implementation is tied to RestorationMixin. That means we will need our own HookRestorableProperty, which is what I wanted to avoid, but I'm OK with going that route. The proposed API would not change except RestorableProperty becomes HookRestorableProperty.

[rrousselGit]

We can try an implements RestorationMixin to see if that is feasible. If not, making a PR to Flutter to separate RestorableProperty from RestorationMixin is a possibility.

[blaugold]

Looking at RestorationProperty._register I doubt implementing RestorationMixin will work. RestorationMixin calls private methods of RestorableProperty to properly register and unregister it.

[jaumard]

We can try an implements RestorationMixin to see if that is feasible. If not, making a PR to Flutter to separate RestorableProperty from RestorationMixin is a possibility.
Looking at RestorationProperty._register I doubt implementing RestorationMixin will work. RestorationMixin calls private methods of RestorableProperty to properly register and unregister it.

Might worth asking flutter team to do that then by just make the private method public it could be solved

[blaugold]

I don't think there is a good way to build on RestorationProperty and RestorationMixin:

• RestorationMixin is a mixin on State
• RestorationProperty has a getter state, which returns the RestorationMixin it is registered with
• Anything implementing RestorationMixin also has to implement State
• The element of HookWidget cannot implement State.widget because its HookWidget is a StatelessWidget and State.widget has to be StatefullWidget

The problem is that it's all tied to the State class.

[blaugold]

I'm closing this PR since it seems to me that implementing this properly is more trouble than it's worth.