frideosapps / frideos_flutter

An all-in-one Fllutter package for state management, reactive objects, animations, effects, timed widgets etc.
https://pub.dev/packages/frideos
BSD 2-Clause "Simplified" License
189 stars 22 forks source link

[Question] Migration from 0.1.3 to 0.4.1 #4

Closed AlexandreRoba closed 5 years ago

AlexandreRoba commented 5 years ago

Hi Frideos,

I've been using the 0.1.3 for some time now. I started looking at the 0.4.1 but faced an issue you can maybe help me with. to demonstrate this I have reimplemented the default flutter app with a BLOC that looks like this:

class HomePageBloc {
  final StreamedValue<int> _counter;

  HomePageBloc() : _counter = StreamedValue<int>(initialData: 0);

  Stream<int> get counter => _counter.outStream;

  void incrementCounter() {
    _counter.value++;
  }
}

Unforunatelly the counter 0 never show on the widget. the initial Data is always null. Is this expected?

            StreamedWidget(
              stream: _bloc.counter,
              builder: (context, snapshot) {
                if (snapshot.data == null) return Container();
                return Text(
                  '${snapshot.data}',
                  style: Theme.of(context).textTheme.display1,
                );
              },
            ),

the initial data is not available on the stream. Any sugestion?

Regards,

Alex.

AlexandreRoba commented 5 years ago

This works when using the ValueBuilder... Any reason why it is not working when registering to the outstream directly with the streamedwidget or StreamBuilder?

frideosapps commented 5 years ago

This works when using the ValueBuilder... Any reason why it is not working when registering to the outstream directly with the streamedwidget or StreamBuilder?

Hello. If you are using StreamedWidget or StreamBuilder you need to give to the initialData parameter the value of the streamedObject, like in this example:

// View
ValueBuilder<int>(                
  stream: bloc.count, // no need of the outStream getter with ValueBuilder
  builder: (context, snapshot) =>
    Text('Value: ${snapshot.data}'),
  noDataChild: Text('NO DATA'),
),
RaisedButton(
    color: buttonColor,
    child: Text('+'),
    onPressed: () {
      bloc.incrementCounter();
    },
),

// As an alternative:
//
// StreamedWidget<int>(
//    initialData: bloc.count.value
//    stream: bloc.count.outStream,
//    builder: (context, snapshot) => Text('Value: ${snapshot.data}'),
//    noDataChild: Text('NO DATA'),
//),

My advise is to use the ValueBuilder widget for all the classes of this library that implements the StreamedObject interface and use the StreamBuilder for the StreamedTransformed (this is going to change in the future, but it is not a priority). The code is more clean and you don't need to pass the outStream getter but just the object itself as an argument to the stream parameter (maybe this parameter should be called 'streamed' but I didn't want to change more than I did). Any suggestions, as always, are welcome:-)

frideosapps commented 5 years ago

@AlexandreRoba issue solved?

AlexandreRoba commented 5 years ago

Hi @frideosapps thanks for the answer. Yes it does. First thanks for comming up with this package, I believe it is great and it is a big helper to simplify a lot of boiler code in the BLOc. It is awesome :p

I do have couple of comments on the direction the latest version is taking :) I can understand the reason behind the ValueBuilder and the exposition of the StreamedValue Interface to the BLOC I would however avoid to do that. One of the good think amongst the other with the first implementation was the fact that you could inline replace a BLOC/VanillaOrWhatever with any BLOC/Frideos BLOC component. I would keep the StreamedObject as being part of the details of the BLOC implementation. By exposing the StreamedValue as the output of the bloc you are making your BLOC relying on an implementation of the StreamedObject.

IMHO the BLOC interface should be relying on Stream and Sink only. Like for the rx dart implementation, Brian explicitly deviated from the RX implementation to make sure that observables are still stream. :) I believe the value should always be the latest version on the outStream and if nothing has been pushed to the outStream and an initialdata is set, then the value and the latest entry in the outStream should be the initial data. Let me know what you think of this. I'll be happy to talk more on it if you want

frideosapps commented 5 years ago

Thanks for the feedback @AlexandreRoba, very appreciated.

The main reason for the package was initially just to simplify the implementation of the BLoC pattern, trying to be as much compliant as possible to the standard implementation. However, I think it was still not easy enough and not so complete and started to think there should be some compromise to do. My aim is to make something as easy as ScopedModel but taking advantages of the streams. Not a task so easy.

Starting from the v0.4.0 the package has an interface to implement a basic state management out of the box: with the AppStateModel interface you can define a state model and let the widget access it by using the associated AppStateProvider. E.g. I used this app state to make a layer above the MaterialWidget and change the theme of the app by using a StreamedValue as a property of the app state (you can see the "Theme changer" example). Here you can even intantiate the BLoCs, becoming the data layer between them. In this case no need of a BLoC provider, you can just use the above mentioned AppStateProvider to provide the BLoCs.

The ValueBuilder is a "package specific" widget to use along with the classes that implement the StreamedObject interface. This is a step further to make something "easier" to adopt and to manage. I agree with the fact that per "standard" implementation you should rely on stream and sink only, and in this case you can just use the StreamBuilder (or the StreamedWidget) and using the outStreamgetter as per v0.1.3.

I made a gist to show you the differences: counter_demo

Currently I am working on a more robust example to show a thorough implementation of the library in a complete app (simple example are not so useful anymore), but I it will take some time.

I am open to evaluate any ideas and advices, so feel free to talk about the package.

AlexandreRoba commented 5 years ago

Hi @frideosapps I will have a look at the AppStateModel and will get back to you on this. Looks interesting. Thanks for the gist.

Meanwhile I looked at the implementation of the SreamedValue. Do you see any issue in using as internal StreamController a BehavioralSubject instead of the default StreamController broadcast? By doing so the outStream will have the initialData set as the first stream entry when subscribing to it. The StreamValue constructor will then look like this:

class StreamedValue<T> implements StreamedObject<T> {
  StreamedValue({this.initialData}) {
    stream = BehaviorSubject<T>();

    stream.stream.listen((e) {
      _lastValue = e;
      _onChange(e);
    });

    if (initialData != null) {
      _lastValue = initialData;
      stream.add(_lastValue);
    }
  }
...
}

This will allow to expose the outStream in the Bloc and have the first element in the stream being the initial value if set with the initialData.

frideosapps commented 5 years ago

Hello @AlexandreRoba , the BehaviorSubject was used until the v0.4.0 exposing the outStream as Observable, I switched to a plain StreamController.broadcast exposing it as a more simple Stream and take advantage of the initialData parameter to avoid to miss the first event on pushing a new page, thinking it was simple way to do the same (and to reduce dependencies over other packages). Currently the BS is used only in the StreamedTransformed that I am not quite sure it is of some utility and even started thinking to remove it. I can consider to reimplement it as the old way, reverting from StreamController.broadcast to BS, but I'd like to maintain the ValueBuilder as a widget package specific (in this case renaming the parameter from stream to streamed to highlight the difference) depending on the StreamedObject interface, letting the StreamBuilder/StreamedWidget to be used in a more "experienced" way using the outStream getter. I think this is a necessary step to make the package easier to adopt for beginners/intermediate users that don't need advanced features but want use this package.

AlexandreRoba commented 5 years ago

Hi @frideosapps, can we consider using the same approach that has been used by RXCommand (https://pub.dartlang.org/packages/rx_command) and especially: MAYBE BREAKING CHANGE in 2.0.0: Till now the results Observable and the RxCommand itself behaved like a BehaviourSubjects. This can lead to problems when using with Flutter. From now on the default is PublishSubject. If you need BehaviourSubject behaviour, meaning every new listener gets the last received value, you can set emitsLastValueToNewSubscriptions = true when creating RxCommand.?

frideosapps commented 5 years ago

Hello@AlexandreRoba, yesterday I released the new versions of the packages with the BS (both frideos v0.5.0 and frideos_core 0.4.2), along with the modifications to the name of the stream parameter of the ValueBuilder widget to streamed (both frideos and frideos_light packages). I never used RxCommand so I don't know clearly how it works in general but it is a feature it could be added. Do you think is it necessary to be able to switch between PublishSubject and BehaviorSubject?

frideosapps commented 5 years ago

Hello, I'm closing this for now, feel free to reopen it.