Observer component and MobX Hooks integration example app

[juancastillo0]

Closes #26

Hi,

As discussed in the issue, this PR implements an ObserverComponent within the framework. The code is similar to some of the sections for InheritedComponents, there is a _updateObservers that assigns the _observerElements from the parent and registers the element if it itself is an ObserverElement. The ObserverElement methods willRebuildElement, didRebuildElement and didUnmountElement are already executed in the build process. At the moment, they are required to be implemented (do not have a default implementation). However, it could be best to leave an empty function implemented?

I created a mobx_hooks_experimental app within the experimental folder. It contains the same logic as before for hooks and mobx tracking, but using the new ObserverComponent infrastructure. The MobXHooksObserverComponent is used only once at the top of the component tree in main.dart and all child elements are being tracked (they can use hooks and they subscribe automatically to observables).

I think I would like to make some tests for the framework and for the mobx_hooks part too. What do you think we should do about the MobX and Hooks integration? At the moment it is some kind of example app, however, it could be a complete package.

Thanks!

[juancastillo0]

Hi,

The last commits add tests, along with small changes, to the framework, and a test with some refactoring to the hooks part.

The last couple of changes include overriding the update method from ObserverElement to make sure the children are rebuilt. And now _observerElements is cleared in unmount to make sure the execution of didUnmountElement has the registered observers.
The tests perform updates to the Component tree and listen to the execution of the methods in ObserverElement. There is a stage in which there are two ObserverComponents listening together. All the methods, including didUnmountElement are executed in the test. The rebuilding of the tree is performed localized and globally.

In the hooks part, I did some refactoring so that the jaspr part is very lean, the hooks logic can be used in a Dart only context (there is a test with multiple hooks). The jaspr component uses all methods in its ObserverElement, you can view it in the experiments/mobx_hooks/lib/mobx_hooks/jasper_observer.dart file.

I believe most of the things are ready, specially on the framework part.

Please let me know what do you think and if there is something more I can do.

Thanks!

[schultek]

Thanks for this, looks very cool. I think this will be a very valuable addition.

I changed the target branch to develop, please check that it works with the new rendering system

[juancastillo0]

Hi thanks for reviewing

I just committed the proposed changes to check _observerElements!.isNotEmpty before iterating and, with the new rendering system, I had to change the expected events in the tests for ObserverComponent. Before, willRebuildElement and didRebuildElement was not being executed with TextElement as argument, only the didUnmountElement. In the last phase of the test I change the text in the Text component and the willRebuild and didRebuild are not called with the TextElement as argument. They are called only once when the element is created.

Also the TestComponentController.rebuild() was not rebuilding the root component. I had to change it to (I did not commit this change):

extension PumpTestComponent on ComponentTester {
  Future<TestComponentController<T>> pumpTestComponent<T>(TestComponent<T> component) async {
    await pumpComponent(component);
    Element? testElem;
    binding.rootElement!.visitChildren((element) {
      element.visitChildren((element) {
        testElem ??= element;
      });
    });
    return TestComponentController(testElem!);
  }
}

And the TestDomBuilder was missing a @override void skipContent(DomNode node) {}.

[schultek]

Ok thanks, I will look at the bugs you mentioned.

About the Text component, it technically does not rebuild or build at all, since it has no build method and no children. But I can make it that the observers are still called.

[juancastillo0]

Yes, I believe it would make sense that it does not rebuild, like a cost Component or one whose reference is the same. However, having the build event as well as the unMount I think does makes sense and the behavior would be the same as a const Component. This is the current situation. It would be weird not having the build, since there is an unmount event afterwards, but maybe for that one should listen to the mount event (not provided by ObserverComponent) and not the build one.

For hooks or MobX it is not that important since the most important elements to track are the ones that have build methods where one uses hooks or observables, but maybe other applications of ObservableComponent could use the other elements lifescycle.

Thanks again for reviewing!

[schultek]

When you merge with develop the bugs with the testing library should be fixed.

I thought about the issue with Text and decided I don't want to artificially call the build hooks when the text component infact never rebuilds. As you said for most use-cases this shouldn't matter, since the main focus is to track the build methods of stateless and stateful components. If this is needed in the future this can easily be added, but for now I would just leave it as it is.

[juancastillo0]

Great, thanks!

The test are working now. No issues in the merge.