brianegan / scoped_model

A Widget that passes a Reactive Model to all of it's children
BSD 3-Clause "New" or "Revised" License
773 stars 76 forks source link

Merge "provider" and "scoped_model" #61

Open rrousselGit opened 5 years ago

rrousselGit commented 5 years ago

provider is an alternative to inherited widgets management.

Both provider and scoped_model aims at doing the same thing, which provider having a slightly larger vision:

provider is not limited to exposing a Model and works with potentially anything, including BLoC, streams, and others.

On the other hand, scoped_model is a lot more popular than provider.


The state of things is pretty confusing for the community.

scoped_model is not enough for advanced inherited widgets, but provider has a too small reach for peoples to find the package.

Would this make sense to merge both packages?

provider already implements the same features as scoped_model but with different names. provider could expose aliases of its implementations to match scoped_model names to make the merging non-breaking.

brianegan commented 5 years ago

Thanks, @rrousselGit!

Overall, I really like your provider library and definitely think it can be used as a replacement for ScopedModel. That said, I'd just like to quickly list some pros and cons I can think of to help evaluate it as objectively as possible and I'd love to hear much more from the community of people using this library before coming to any conclusions.

Pros

Cons

Questions for the community

rrousselGit commented 5 years ago

My main concern is that currently, we have many duplicates. What the community currently do is, instead of making PR in one repo, they bake their own.

I don't see it as being sustainable. It's confusing and splits the community effort across many packages for no reason.


  • Do most folks who use scoped_model need the additional features of provider, or is scoped_model enough for many apps?

Considering more than half of the points in pros represents an issue on scoped_model, I think the answer is yes.


About breaking changes, I think it is possible to merge provider into scoped_model while being invisible for users.

This is ultimately just widgets with different names. We could replace the implementation of ScopedModel.build/of to simply point to the equivalent provider (ChangeNotifierProvider).

The only technical difference is provider is based on ChangeNotifier, and not just Listenable. This is mainly because Listenable is not disposable, while ChangeNotifier is. That, and the fact that ChangeNotifier implements reporting errors to Flutter.

brianegan commented 5 years ago

What the community currently do is, instead of making PR in one repo, they bake their own.

Agreed. I think the question might become: In that case, should we merge the features of provider into scoped_model instead of deprecating scoped model in favor of a provider?

I'd agree the name provider and the associated Widgets are more appropriately named, IMO. Even the second version of ScopedModel in the Fuchsia codebase uses the name Provider.

Therefore, I think keeping provider as the core package makes the most sense, so long as the merge is transparent to end-users.

To clarify the plan:

This would allow a very nice upgrade path for folks while giving those who need the extra functionality immediate access. We could also collaborate together more on features in the core lib from that point on.

tsun424 commented 5 years ago

Hi @brianegan , I just switched my apps from using flutter_redux to scoped_model because I think scoped_model is much easier. Hopefully, this provider merging won't affect current scoped_model.

BTW, another state management package provide was just released. Too many options in community now, and hope again the state management solution can get relatively unified ASAP.

filiph commented 5 years ago

Great discussion here! I'd like to point to a related discussion.

The "ScopedModel v2" from Fuchsia is now open sourced as https://github.com/google/flutter-provide, and on pub as package:provide. There is great feedback from Remi here: https://github.com/google/flutter-provide/issues/3.

I don't have an answer yet and I'd like Eric and I to ponder our options. I do think Remi's package is fantastic. I also like the simplicity of ScopedModel / provide. For example, I agree with Brian that flutter_hooks is a big dependency.

There is value in choice but there is also value in one (potentially "official") starting point.

Please keep the discussion going.

tsun424 commented 5 years ago

Hi @filiph , you called provide as "ScopedModel v2", I was wondering why you didn't upgrade this original scoped_model to V2 and try to keep backward compatibility, which will give flexibility to community to use a unified package.

Agree with this:

There is value in choice but there is also value in one (potentially "official") starting point.

If provide is considered as an official package, maybe put it into organisation flutter is better?

In one word, relatively unified/stable(understand 100% unify never happens, also should allow different choices) is good for community.

rrousselGit commented 5 years ago

The dependency on flutter_hooks made sense when it was a loner package, as all of my packages complement each other.

But if provider is taking a more official path, then I'm entirely fine with removing the dependency.

halfahad commented 5 years ago

@brianegan @rrousselGit @filiph Any updates on the direction of your efforts in the scoped_model arena. Where should we direct people?

Would be nice to keep the community updated on any future intentions, and excited to see a concerted effort.

xalikoutis commented 5 years ago

The super Marvel - DC power of scoped_model is simplicity. Is understanding docs with snap photographic reading and using with a straight simple way. For more complexity, obfuscating code, no life go for Bloc, redux. So keep it as is

brianegan commented 5 years ago

@halfahad IMO, it makes sense to merge these projects. Remi is working on v2, which removes the dependency on flutter_hooks. Once that's in place and the API has a couple of months to stabilize, I'll encourage folks to move over.

@xalikoutis Thanks for the input. Overall, while Provider offers extra functionality, I think it can be just as simple :)

Here's a translation of the Counter app in the readme to use provider, highlighting the differences in the code comments. It's pretty much the same!

// Extend ChangeNotifier instead of Model. 
// ChangeNotifier is part of Flutter and works the same way as model!
class CounterModel extends ChangeNotifier {
  int _counter = 0;

  int get counter => _counter;

  void increment() {
    _counter++;
    notifyListeners();
  }
}

class CounterApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    // Use the ChangeNotifierProvider Widget instead of ScopedModel
    return ChangeNotifierProvider<CounterModel>.stateful(
      builder: () => CounterModel(),
      child: Column(children: [
        // Use Consumer instead of ScopedModelDescendant
        Consumer<CounterModel>(
          builder: (context, model) => Text('${model.counter}'),
        ),
        new Text("Another widget that doesn't depend on the CounterModel")
      ])
    );
  }
}
hectorAguero commented 5 years ago

@rrousselGit now that provider 2.0 is out, can you update the last example from @brianegan ?

rrousselGit commented 5 years ago

Sure~

// Extend ChangeNotifier instead of Model. 
// ChangeNotifier is part of Flutter and works the same way as model!
class CounterModel extends ChangeNotifier {
  int _counter = 0;

  int get counter => _counter;

  void increment() {
    _counter++;
    notifyListeners();
  }
}

class CounterApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    // Use the ChangeNotifierProvider Widget instead of ScopedModel
    return ChangeNotifierProvider<CounterModel>(
      builder: (_) => CounterModel(),
      child: Column(children: [
        // Use Consumer instead of ScopedModelDescendant
        Consumer<CounterModel>(
          builder: (context, model, _) => Text('${model.counter}'),
        ),
        new Text("Another widget that doesn't depend on the CounterModel")
      ])
    );
  }
}
kbrmimbyl commented 5 years ago

ScopedModel user here. Using it in a pretty complex app and IMHO there’s no need to merge it with Provider (which itself looks awesome. I think I would have used it instead of ScopedModel if it were available in December 2018).

brianegan commented 5 years ago

Thanks for the feedback, @kbrmimbyl!

Hawlitschek commented 5 years ago

Previously ScopedModel User here.

Today I just run in several issues I am not able to explain to myself. I was using a backdrop navigation, managing the current front panel using scoped model. When selecting a front panel that contains a google maps widget, the ScopedModel get killed on iOS in an instant => the app rebuild with the initial scoped model state, resulting in an instant switch back to another panel.

Since the google map wasn't that necessary, I could just remove it.

On an other front panel I used an image grid with a plus button for image selection & upload. Hitting the plus button opens multi_image_picker for choosing images to upload. Whenever coming back from the picker (don't matter if canceling or selecting images) on iOS, the model got killed => The front panel from the inital model state was shown.

In both cases no error messages or stack traces.

Migrating to provider seems to fix the issue.