Provider: [RFC] Should we make a v3.0.0 with name tweaks/constructors swap?

[rrousselGit]

There are a few inconsistencies among the public API

• StreamProvider.builder from the default constructor requests a StreamController instead of a stream. It'd be more logical for the default constructor to request a Stream, and have a named constructor or another class for the StreamController.

• XXProvider.value have inconsistencies in the name used for the value. ChangeNotifierProvider requests a notifier, Provider requests a value, ValueListenableProvider requests a valueListenable. But they probably should all be named value.

• Not an immediate issue, but ProxyProvider.custom has a weird naming issue. It takes both a builder and a providerBuilder argument. Maybe we should globally rename builder to something better (valueBuilder like it originally was?)

These are minor changes but requires a breaking change.

cc @brianegan @filiph

[brianegan]

As discussed over slack, big +1 to StreamProvider having a default constructor with a builder that returns a Stream instead of a StreamController. As discussed, we've already seen example videos that misuse the StreamProvider.value with Firebase, leading to an issue where the Stream will be recreated every time the build method that created the StreamProvider is called.

I also like the idea of making everything value, similar to how everything is consistently named builder.

[rrousselGit]

Technically, the 2 last points can be solved without a breaking change. Since they are based on an optional parameter, we can mark them as @deprecated and keep the previous one.

Here's an example:

  const Provider({
    Key key,
    @required ValueBuilder<T> valueBuilder,
    @deprecated ValueBuilder<T> builder,
    this.dispose,
    this.child,
  })  : assert(builder != null),
        updateShouldNotify = null,
        super(key: key, builder: valueBuilder ?? builder);

Note that with the upcoming non-nullable types, it's likely that we'll need a 3.0.0 anyway.

So maybe we can keep things as is (with the new StreamProvider constructor as a named constructor instead of default) – and do the constructor swap at that time.

[brianegan]

Yah, timing these things is always hard.

It seems like Provider is really just starting to take off since the Google IO talk and the vids and such are just starting to be made, so it might not be a bad time to fix these in a 3.0.0 sooner than later, but that's just my gut feeling.

[rrousselGit]

Another soon to be breaking change is the typedef feature.
Currently, we have 6 different implementations of Consumer due to the lack of optional type parameters.

With typedefs we could do:

typedef Consumer<T> = Consumer6<T, Null, Null, Null, Null, Null>;
typedef Consumer2<T, S> = Consumer6<T, S, Null, Null, Null, Null>;
...

That's cool. But that'd mean that Consumer == Consumer2, which is currently not the case. So another technically (although very minor) a breaking change.

[rrousselGit]

Yeah, I agree.
We should either act before the community starts really using it (we already see quite a few videos), or leave things for later.

[peekpt]

If you do a custom constructor it's very common to use unnamed var on the first parameter

Provider<MyComplexClass>.builder(
  (context) => MyComplexClass(),
  dispose: (context, value) => value.dispose()
  child: SomeWidget(),
)

// for a custom value object
Provider<Bar>.value( 
  Bar(name: 'Hold My Beer'),
  child: Consumer<Bar>.builder(
     (context, value, child) => Text(value),
  ),
);

// for a default constructor class
Provider<Foo>(
  child: Consumer<Foo>.builder(
     (context, value, child) => Text(value),
  ),
);

// for multiple providers
Provider.providers([
    Provider<Foo>.value(foo),
    Provider<Bar>.value(bar),
    Provider<Baz>(),
  ],
  child: someWidget,
);

Maybe it's better to replace _value_ using _object_ like Provider.object(Foo(var), ...

[filiph]

I am torn. All these changes look like great ideas, but I feel it's important to evolve this package with extreme conservatism.

For that reason, I'd suggest that 3.0 isn't published until we have a 2.x that has an alternative approach, and the previous approach is marked with @Deprecated.

So, in case of ChangeNotifierProvider, something like

class ChangeNotifierProvider {
  ChangeNotifierProvider({
    @Deprecated('please use value instead') ChangeNotifier notifier,
    ChangeNotifier value,
    ...
  });
}

In general, consider how deeply this package already is intertwined in people's projects. We don't want a large set of breaking changes a few weeks after the last one.

Brian is right that there hasn't been time to anyone to _really_ pick this up, but it's still a bad developer experience to be rewriting code this close to writing it. People assume this is going to be the norm.

[rrousselGit]

Personally, I don't think this is needed. The changes are mostly cosmetic.
I don't think the discussed changes justify a breaking change that early in the adoption of the package, for the same reasons that Filip described.
And considering the future of Dart, we _will_ have opportunity to make these changes later.

On the other hand, StreamProvider definitely needs a constructor that takes a (context) => Stream<T>. But that can be done in a non-breaking way.

[delay]

My two cents worth as a flutter newbie... Don't worry about breaking changes causing problems. Flutter is very new for most of us. Honestly I would rather see a breaking change, at least I know its being actively developed. It does look like Provider will be the way to go moving forward. I am currently building my first production app in flutter and I have had half a dozen breaking changes since I started. As long as its documented, it is usually really quick to fix them. The whole reason I found this issue is because StreamProvider is not returning a stream and its very confusing. It would be nice to see some code example as to how to make it return a stream instead of a stream controller if this isn't being fixed now. It's the main feature I was looking for. I say fix stuff now, it becomes a bigger issue to fix it later once it's used by many more apps. Breaking changes are always problematic to deal with, better to handle them ASAP before you gain more traction and more people have to update their apps to whatever you end up with. Also it's only in the last few weeks that this project is getting noticed, so I doubt there are too many apps in production yet. So easier to make changes while you are building an app then later when in maintenance mode.

[rrousselGit]

I appreciate the feedback @delay
I agree that StreamProvider needs a way to create a Stream instead of StreamController.

But do you think it is necessary for that feature to be on the default constructor?

We may instead have:

StreamProvider(
  builder: (_) => StreamController(),
)

StreamProvider.value(
  stream: Stream<T>,
)

StreamProvider.custom(
  builder: (_) => Stream<T>(),
)

I know that it would still be confusing for StreamProvider.custom (or whatever its name will be) not to be the default constructor. But that would not be breaking.

[delay]

Ok my opinion as a newbie... If I see StreamProvider, my first thought is ok cool let me use that to get a stream and supply it to my StreamBuilder. I then do this and it gives me an error. So then I would head over to the Provider documentation and hopefully find an example of how to implement what I need to do. If I can't find it there or via a google search then I am thinking I guess I can't really use this package because I can't get it working. Right now I have spent a few hours searching and haven't found any code examples of how to accomplish what I need... a stream from Provider. So I think this doesn't solve the full problem of handling state in my app. Honestly provider looks great otherwise and it is very easy to understand compared to alternatives, so it will probably become the default state management solution. Right now I think flutter needs one simple state management package, that's it's main weakness. Coming from another language this has been by far the hardest thing for me to understand about flutter. Everything else has been really straightforward except state management.

Right now, You are seeing videos that implement StreamProvider incorrectly, you have people on your slack asking about it. If it were me I would do the breaking change now it only becomes worse later and if it's confusing now, it won't get better later as more people move to flutter who aren't early adopters. Early adopters expect breaking changes. I would just provide a simple example for how to make the code change... This is really easy to handle as a developer. In the end you fix what isn't obvious with your API. You also have a better chance at becoming the default state management solution because it just works and is obvious.

Every time I update flutter, something breaks in some package, but it's always easy to fix. Breaking changes are nothing for me to deal with. Not understanding how to use an API is my biggest problem. Right now I am trying to figure out the best way to build my app and having a good easy to understand package is much more important to me. Provider is a package, so if I couldn't deal with a breaking change right now, couldn't I just run the older version of the package? I guess I don't get why a breaking change is a big deal.

For an api change. This is what would make the most sense to me:
Make a stream the default that is returned. Do something like this to return a controller.

StreamProvider.value(
  controller: Stream<T>,
)

It would take 2 minutes to change this breaking change.

P.S. I discovered your package from the fireship.io videos.

[timsneath]

My own opinion, for what it's worth. I strongly suspect that over 99% of your ultimate customers aren't using provider yet (and 80% of your ultimate customers haven't even discovered Flutter yet!) And we've already seen with Flutter how hard it is to make breaking changes once code is ossified in videos, labs, tutorials etc. So if you have a clear signal that it's possible to make this cleaner / simpler / more ergonomic, I'd agree with @delay that making changes sooner rather than later is ideal.

The key is to provide simple examples of the breaking changes -- "if your code looked like X, make it look like Y". Make it trivial to update and you won't have any problem getting folk to follow along.

[rrousselGit]

To add more arguments to the list, I talked to a bunch of important members of the community, and they brought up two interesting points:

• There was a survey about breaking change for Flutter, and the answers said they welcomed a little break for the better good.
• The change is very simple, and can potentially be automated with tools such as https://github.com/Workiva/dart_codemod

---

I'm still doubtful whether this change is _truly_ necessary.

But at least 2 persons saw the same video and reported the same issue.
Plus, I do agree that the changes suggested on StreamProvider by @brianegan are for the best.

Should we consider it case closed and proceed to make the changes then?

[peekpt]

One thing I don't get it:
An app with master and detail views.
I want to supply a DAO class for each view.
Should I use a Multiprovider right after main() or should I use individual Providers in the views class?

I want to instatiate the classes only when the view is loaded. My question is do they get instantiated already if I put it on the top of the app or do they have like lazy instantiation

[filiph]

For what it's worth, I am still torn between the two alternatives, but hearing from all of you, I'm much closer to "let's do the breaking change now" than before. I'm deferring to Remi's and others' judgement.

[chimon2000]

As a consumer, I am fine with the breaking changes so long as there is a migration path clearly documented in the changelog. Just by definition of semantic version, I expect breaking changes for a major upgrade.

Taken from semver.org:

Given a version number MAJOR.MINOR.PATCH, increment the:

1. MAJOR version when you make incompatible API changes,
2. MINOR version when you add functionality in a backwards-compatible manner, and
3. PATCH version when you make backwards-compatible bug fixes.

[rrousselGit]

So, I've listened to the suggestions and made a v3 branch.

It should be "ready" now. The only remaining question is whether we want #105 or not.

Feel free to comment here or on #95 if you have any concern or see a mistake.

If nothing pops up for a week, I'll make a release. In the mean time I'll make an article and examples.