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

ObservableList is not detected by Observer #851

Closed
subzero911 opened this issue Aug 12, 2022 · 15 comments
Closed

ObservableList is not detected by Observer #851

subzero911 opened this issue Aug 12, 2022 · 15 comments

Comments

@subzero911
Copy link
Contributor

subzero911 commented Aug 12, 2022

I have the following code, where mapObjects is an ObservableList:
image
Output in the console:
image

The declaration of ObservableList
image

I don't need to wrap it into Observable<T>, as I never change the actual list reference, but the list content only:
image
I believe an Observer should be triggered and cause the rebuild, but it doesn't happen.
Sorry for comments in Russian, just ignore them.

May be related to this discussion #765

@subzero911
Copy link
Contributor Author

I also tried to wrap it into Observable<ObservableList<MapObject>> and mutate the inner ObservableList, but Observer still does not rebuild (while it now sees the Observable inside).
But, when I use Observable<List> (plain list) and change it like mapObjects.value = [clusterizedCollection], everything works fine.

@giri-jeedigunta
Copy link
Contributor

giri-jeedigunta commented Sep 8, 2022

I cannot figure out your full code ... but I think you need to annotate theObservableListwith @observable see this example from TODOs - lines 22 - 24

ObservableList<Todo> todos = ObservableList<Todo>();

@subzero911
Copy link
Contributor Author

subzero911 commented Sep 8, 2022

No, if I annotate it with @observable, it will mean that Observer now reacts to re-assigning the list reference, like todos = [...todos, newItem];
But I'm not going to do this. I want to mutate an existing list itself, like todos.add(newItem) (not creating a new list instance, but changing the existing list).
The problem is that Observer does not detect this move for some reason.

Also, I don't use a code generation in my project and avoid any annotations.
@observable
ObservableList<Todo> todos;
is just a pretty shortcut for Observable<ObservableList<Todo>> todos

@giri-jeedigunta
Copy link
Contributor

Got it!

@subzero911
Copy link
Contributor Author

subzero911 commented Sep 8, 2022

Also, to recreate a list like todos = [...todos, newItem]; you don't even need ObservableList.
It's enough to have a plain Dart List

@observable
List<Todo> todos;

The ObservableList power is to send notification on any change (add / remove / clear, etc.), so Observer will receive it.
So I can change any inner collections in my model, without copyWith, and Observer will rebuild.

Moreover, it's one of the reasons why I ever use MobX.
Because Flutter provides an "observable-like" solution out of the box, called ValueListenable.
But there's nothing like ObservableList in Flutter.

@deadsoul44
Copy link

May be add method of ObservableList is not implemented so it is not reactive, i.e. acting like plain add method. Need to check the source code of ObservableList.

@deadsoul44
Copy link

It is implemented


so there should be a bug.

@subzero911
Copy link
Contributor Author

subzero911 commented Sep 8, 2022

May be add method of ObservableList is not implemented so it is not reactive, i.e. acting like plain add method. Need to check the source code of ObservableList.

according to this topic #765
Observer does not notice the ObservableList only when it is passed to the parameter of the child widget.
But it notices regular @observable or Observable<T> variables, passed to the child widget.

@deadsoul44
Copy link

In that case, builder has a new context. There should be a new observer. In other words, there will be no reaction if there is not an observer in the immediate context.

@subzero911
Copy link
Contributor Author

subzero911 commented Sep 8, 2022

But I cannot have an Observer inside of YandexMap, because it's a side-package. I cannot get into its source code and add an Observer.

Also, as I mentioned above, Observable<T> is perfectly tracked by Observer even if not in immediate context.
So there should be a bug in Observer implementation, why it differently handles the Observable<T> and ObservableList

@pr-1
Copy link
Contributor

pr-1 commented Sep 8, 2022

Can you provide the minimal reproducible code as a gist ? I don't think there is any bug in mobx here. There must be some missing implementation.

@subzero911
Copy link
Contributor Author

subzero911 commented Sep 9, 2022

Hi @pr-1. I prepared a minimal app, showing the problem.

https://github.com/subzero911/mobx_observableList_bug_demo/tree/master/lib

If I directly use ListView inside of Observer, everything works fine.
But if I pass ObservableList into child widget, which has a ListView inside, Observer does not track this ObservableList anymore.

But if I would use Observable<List> and re-assign a new list to it, Observer tracks the change as expected, despite I pass it into child widget too.
I also added this variant into separate branch, so you can compare: https://github.com/subzero911/mobx_observableList_bug_demo/tree/observableT_variant/lib

@pr-1
Copy link
Contributor

pr-1 commented Sep 11, 2022

Thanks for the code @subzero911
I have tried to explain what might be going wrong here.

Observer(builder: (_) {
              print('rebuilt');
              return SizedBox(
                width: 1024,
                height: 512,
                child: ChildWidget(
                  heartsList: controller.heartsList,
                ),
              );
            }),
class ChildWidget extends StatelessWidget {
  const ChildWidget({super.key, required this.heartsList});
  final ObservableList<String> heartsList;

  @override
  Widget build(BuildContext context) {
    return ListView.builder(
        scrollDirection: Axis.horizontal,
        itemCount: heartsList.length,
        itemBuilder: (context, index) {
          return SizedBox(
            width: 112,
            height: 48,
            child: ListTile(
              title: Text(heartsList[index]),
            ),
          );
        });
  }
}

The reactive context here will change and mobx will throw No observables found because Child widget creates its own new ObservableList variable which will have 3 hearts when it is built first time.
This new ObservableList is used by the ListView.
Please correct me if I am wrong here @pavanpodila

Instead make the child widget expect a List that is built from the ObservableList using controller.heartsList.toList() -

Observer(builder: (_) {
              print('rebuilt');
              return SizedBox(
                width: 1024,
                height: 512,
                child: ChildWidget(
                  heartsList: controller.heartsList.toList(),
                ),
              );
            }),
class ChildWidget extends StatelessWidget {
  const ChildWidget({super.key, required this.heartsList});
  final List<String> heartsList;

  @override
  Widget build(BuildContext context) {
    return ListView.builder(
        scrollDirection: Axis.horizontal,
        itemCount: heartsList.length,
        itemBuilder: (context, index) {
          return SizedBox(
            width: 112,
            height: 48,
            child: ListTile(
              title: Text(heartsList[index]),
            ),
          );
        });
  }
}

This is the reason why it is working as expected on the other branch . You can test it by making the List inside Child Widget a Observable like final Observable<List<String>> heartsList; on https://github.com/subzero911/mobx_observableList_bug_demo/tree/observableT_variant/lib
This is equivalent to master branch code and it will no longer detect changes from controller.

Similarly for YandexMap try using this because YandexMap is not expecting an ObservableList but a List

Observer(builder: (_) {
              print('rebuilt');
              return SizedBox(
                width: 1024,
                height: 512,
                child: YandexMap(
                  mapObjects: _placesController.mapObjects.toList(),
                ),
              );
            }),

@subzero911
Copy link
Contributor Author

subzero911 commented Sep 11, 2022

Thanks, that actually works!
So the idea behind is that I passed the reference to original ObservableList to the child widget, and an Observer coudn't see any observables inside (because it's in a child's context, not in a current context). And it expects that I will create some Observer inside a ChildWidget and intercept the ObservableList.
But if I pass a List copy to the child, an Observer now can see the original ObservableList modifying, in its context.
So there's no error in MobX, but I used it wrong.
Your code snippet should be in the ObservableList documentation, as it's not evident at all.

@giri-jeedigunta
Copy link
Contributor

One of you should really add this to the docs... learned a bunch of things in this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants