Skip to content
This repository has been archived by the owner on Jul 9, 2020. It is now read-only.

Emitted state not received by CubitBuilder #52

Closed
saschaernst opened this issue Jul 5, 2020 · 20 comments · Fixed by #57
Closed

Emitted state not received by CubitBuilder #52

saschaernst opened this issue Jul 5, 2020 · 20 comments · Fixed by #57
Assignees
Labels
bug Something isn't working waiting for response Waiting for follow up
Projects

Comments

@saschaernst
Copy link

saschaernst commented Jul 5, 2020

Describe the bug
When emitting a state after awaiting a call that was itself not asynchronous (i.e. reading from a repo that has the requested data already cached in memory and does not need to read it from disk again), the new state is not received by a listening CubitBuilder instance.

Hint:
When I follow the emit call in the problematic case, the _AsyncBroadcastStreamController has no subscriptions, which I guess is a problem... There is probably a racing condition when setting these things up.

When not using any await, the new state is the first thing the CubitBuilder gets, never seeing the initial state in the first place. When awaiting a really asynchronous operation, both states are received as expected.

To Reproduce
here is a minimal piece of code to demonstrate the problem:

import 'package:flutter/material.dart';
import 'package:flutter_cubit/flutter_cubit.dart';
import 'package:cubit/cubit.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MaterialApp(
        title: 'Flutter Demo',
        home: CubitProvider(
            create: (BuildContext context) => TestCubit(),
            child: MyHomePage(title: 'Flutter Demo Home Page')),
      );
}

class MyHomePage extends StatefulWidget {
  MyHomePage({Key key, this.title}) : super(key: key);

  final String title;

  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  void didChangeDependencies() {
    super.didChangeDependencies();

    context.cubit<TestCubit>().init();
  }

  @override
  Widget build(BuildContext context) => Scaffold(
        body: Center(
          child: CubitBuilder<TestCubit, String>(
            builder: (context, state) => Text(
              '$state',
              style: Theme.of(context).textTheme.headline4,
            ),
          ),
        ),
      );
}

class TestCubit extends Cubit<String> {
  TestCubit() : super("Waiting...");

  Future<void> init() async {
    final data = await _doSomething(); // works when commented out
    emit(data);
  }

  Future<void> _doSomething() async {
    return 'Works!';
  }
}

Expected behavior
The emitted state should be received by the CubitBuilder instance no matter if the call leading up to the emit is synchronous or awaiting something synchronous or asynchronous

@felangel
Copy link
Owner

felangel commented Jul 5, 2020

Hi @saschaernst 👋
Thanks for opening an issue!

In the example you've provided, when you emit without the 1 second delay the state change happens before CubitBuilder has subscribed to the cubit. Emit pushes a new state as soon as it is called so this is expected behavior. I would recommend refactoring your example slightly:

import 'package:flutter/material.dart';
import 'package:flutter_cubit/flutter_cubit.dart';
import 'package:cubit/cubit.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: CubitProvider(
        create: (_) => TestCubit()..init(),
        child: MyHomePage(),
      ),
    );
  }
}

class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: CubitBuilder<TestCubit, String>(
          builder: (context, state) {
            return Text(
              '$state',
              style: Theme.of(context).textTheme.headline4,
            );
          },
        ),
      ),
    );
  }
}

class TestCubit extends Cubit<String> {
  TestCubit() : super("Waiting...");

  void init() => emit("It works!!!");

  @override
  void onTransition(Transition<String> transition) {
    print(transition);
    super.onTransition(transition);
  }
}

You can set a breakpoint or add a print statement inside of CubitBuilder and you'll see that the transition occurs before CubitBuilder has been built.

If you really want to guarantee CubitBuilder sees the initial state for 1 frame (not sure why you would) then you can call init via a postFrameCallback like:

import 'package:flutter/material.dart';
import 'package:flutter_cubit/flutter_cubit.dart';
import 'package:cubit/cubit.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: CubitProvider(
        create: (_) => TestCubit(),
        child: MyHomePage(),
      ),
    );
  }
}

class MyHomePage extends StatefulWidget {
  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  void initState() {
    super.initState();

    WidgetsBinding.instance.addPostFrameCallback((_) {
      context.cubit<TestCubit>().init();
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: CubitBuilder<TestCubit, String>(
          builder: (context, state) {
            return Text(
              '$state',
              style: Theme.of(context).textTheme.headline4,
            );
          },
        ),
      ),
    );
  }
}

class TestCubit extends Cubit<String> {
  TestCubit() : super("Waiting...");

  void init() => emit("It works!!!");

  @override
  void onTransition(Transition<String> transition) {
    print(transition);
    super.onTransition(transition);
  }
}

In any case, the change will happen so fast you won't most likely won't see it. Let me know if that helps clarify things. If you have outstanding questions it would be beneficial for you to provide a more realistic sample to illustrate what you're trying to accomplish, thanks! 👍

@felangel felangel self-assigned this Jul 5, 2020
@felangel felangel added question Further information is requested waiting for response Waiting for follow up labels Jul 5, 2020
@felangel felangel added this to To do in cubit via automation Jul 5, 2020
@saschaernst
Copy link
Author

saschaernst commented Jul 5, 2020

As I mentioned in my description, the edge case is the situation when you are awaiting a call before the emit which can be synchronous or asynchronous (i.e. reading from repo that might have cached data in memory or has to read it from disk first).

Example works when called for the first time, every other times it gets in trouble as described above:

String foo;

Future<String> getString() async {
  if(foo == null) {
    foo = await doSomethingToGetAStringAsync();
  }

  return foo;
}

When I don't have an await call at all, the state change happens first and CubitBuilder shows the new state when being build, that's fine.
When there is at least a Future.delayed(Duration(seconds:0)) awaited, it also works as expected in the usual asynchronous way.

Only in the first case the Builder is able to read the first state, then the emit happens but CubitBuilder is not notified about the second as it has no subscriptions yet. There seems to be a gap between running the builder for the first time and having it wired up correctly in exactly that case.

@saschaernst
Copy link
Author

The addPostFrameCallback fixes the problem, but feels like a hack for a problem, that should not exist in the first place

@felangel
Copy link
Owner

felangel commented Jul 5, 2020

@saschaernst I'm not sure I understand. If your repo has cached the state and returns synchronously the CubitBuilder will still build with the cached state (it will just skip the initial loading state).

@saschaernst
Copy link
Author

saschaernst commented Jul 5, 2020

No, it doesn't. If the init method containing the emit awaits a method which in fact doesn't do anything async (i.e. data is already cached), the weird behaviour shows up. I updated my example code to show the following:
After putting the breakpoints in front of the builder and emit, the following happens:

  1. builder gets called with 'waiting' (that's good)
  2. emit gets called within the init function (also good)
  3. builder doesn't get the new state and when you follow emit down to the _AsyncBroadcastStreamController, the debugger shows you that it doesn't have any subscriptions.

So the builder reads the initial state, but cannot receive the new one when it is emitted later.

If you have no await in the init call at all, in fact the builder gets build after the emit and it directly reads the new state 'works'.
Somehow an in some cases unused 'await' confuses the system

@felangel
Copy link
Owner

felangel commented Jul 5, 2020

Check out this example:

import 'package:flutter/material.dart';
import 'package:flutter_cubit/flutter_cubit.dart';
import 'package:cubit/cubit.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: CubitProvider(
        create: (context) => TestCubit(Repository())..fetch(),
        child: MyHomePage(),
      ),
    );
  }
}

class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: CubitBuilder<TestCubit, Data>(
          builder: (context, state) {
            return Center(
              child: Column(
                mainAxisAlignment: MainAxisAlignment.center,
                children: <Widget>[
                  Text(
                    '${state.value}',
                    style: Theme.of(context).textTheme.headline4,
                  ),
                  Text(
                    'last updates: ${state.lastUpdated}',
                    style: Theme.of(context).textTheme.caption,
                  ),
                ],
              ),
            );
          },
        ),
      ),
      floatingActionButton: FloatingActionButton(
        child: Icon(Icons.refresh),
        onPressed: () => context.cubit<TestCubit>().fetch(),
      ),
    );
  }
}

class TestCubit extends Cubit<Data> {
  TestCubit(this._repository) : super(Data('waiting'));

  final Repository _repository;

  Future<void> fetch() async {
    final data = await _repository.getData();
    emit(data);
  }
}

class Repository {
  Data _cachedData;

  Future<Data> getData() async {
    if (_cachedData != null) return _cachedData = _cachedData.copyWith();
    await Future<void>.delayed(const Duration(seconds: 1));
    return _cachedData = Data('it works');
  }
}

class Data {
  Data(this.value, [DateTime lastUpdated])
      : lastUpdated = lastUpdated ?? DateTime.now();

  final String value;
  final DateTime lastUpdated;

  Data copyWith({String value}) {
    return Data(value ?? this.value, DateTime.now());
  }
}

The first load happens from "network" and subsequent requests (triggered by tapping the floating action button) are read from "cache". You can see the last updated timestamp change which indicates that the CubitBuilder is in fact rebuilding.

Let me know if that helps 👍

@saschaernst
Copy link
Author

As long as you always have this delay it surely works, but in real code you would not have an asynchronous delay of any kind (network, disk access, etc) on every call, as it would be retrieved from cache from the second time on, but only the first one.

Please have a look at my example method. It is only really asynchronous on the first call. Now imagine the following situation:

  • you have to pages with their individual cubits
  • both cubits access the same repo and retrieve their data from there in an init function
  • as you don't know which page is accessed first, the data retrival is async so the repo can check if it has to load something first and cache the data (like in my example method getString())
  • so the first page being loaded retrieves the data by its cubit asynchronously as it has to be loaded by the repo first
  • the second page's cubit, when loaded, uses the same call to the repo with await getString(), but the data is already there and returned synchonously, so it is not really so slow that the new state arrives savely when the builder is completely setup and waiting for new states to arrive
  • somehow it is send out right in the middle when the builder already read the old state but is not wired up to receive new ones yet
    ====> BOOOOOOM!!!!
  • for some reason the call is not really synchronous and with that faster than the builder, so the builder only reads the new state
  • and it is not really asynchronous

@felangel
Copy link
Owner

felangel commented Jul 5, 2020

My example doesn’t always have a delay. My repository method is also only awaiting on the first request. Subsequent requests return the data from cache. The CubitBuilder will always receive the latest state.

I think the issue you’re facing is if your cubit emits a state s where s == state (in other words the newly emitted state is equal to the current state of the cubit) then no state change will happen so there will be no updated state for the CubitBuilder to receive. You can verify this by overriding onTransition and printing the transitions as they occur.

@saschaernst
Copy link
Author

saschaernst commented Jul 5, 2020

I will not use a stateless widget anymore, as calling the init within its build method probably messes with the flutter system. I am just wondering why I have to use addPostFrameCallback in initState or didChangeDependencies.

// answering your last comment
I checked my states, they are not the same. Again: the state is emitted goes through a a lot of stages down to the _AsyncBroadcastStreamController but that one has no subscriptions yet

In the meantime I came to believe that it is not a cubit bug, but some weird edgecase of streams. I take your fix thankfully and move on

@saschaernst
Copy link
Author

saschaernst commented Jul 5, 2020

You will run into my troubles if you would navigate to another page with a cubit reading from the same repo, trust me. Manually calling init works as the Builder had time to set up properly

@felangel
Copy link
Owner

felangel commented Jul 5, 2020

My getData doesn’t always have a one second delay. If cachedData is not null it is returned immediately with no delay. You shouldn’t need to override didChangeDependencies in this case and you also shouldn’t need addPostFrameCallback.

The latest example I shared demonstrates how I would handle this. I can add a second page 👍

@saschaernst
Copy link
Author

Sorry, I misread your code. yes, try the second page, please

@felangel
Copy link
Owner

felangel commented Jul 5, 2020

I've added a details page 👍

import 'package:flutter/material.dart';
import 'package:flutter_cubit/flutter_cubit.dart';
import 'package:cubit/cubit.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return CubitProvider(
      create: (context) => TestCubit(Repository())..fetch(),
      child: MaterialApp(
        home: MyHomePage(),
      ),
    );
  }
}

class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text('Home')),
      body: Center(
        child: CubitBuilder<TestCubit, Data>(
          builder: (context, state) {
            return Center(
              child: Column(
                mainAxisAlignment: MainAxisAlignment.center,
                children: <Widget>[
                  Text(
                    '${state.value}',
                    style: Theme.of(context).textTheme.headline4,
                  ),
                  Text(
                    'last updates: ${state.lastUpdated}',
                    style: Theme.of(context).textTheme.caption,
                  ),
                  FlatButton(
                    child: Text('Details'),
                    onPressed: () {
                      Navigator.of(context).push<void>(MyDetailsPage.route());
                    },
                  )
                ],
              ),
            );
          },
        ),
      ),
      floatingActionButton: FloatingActionButton(
        child: Icon(Icons.refresh),
        heroTag: 'fab1',
        onPressed: () => context.cubit<TestCubit>().fetch(),
      ),
    );
  }
}

class MyDetailsPage extends StatelessWidget {
  static Route route() {
    return MaterialPageRoute<void>(builder: (_) => MyDetailsPage());
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text('Details')),
      body: Center(
        child: CubitBuilder<TestCubit, Data>(
          builder: (context, state) {
            return Center(
              child: Column(
                mainAxisAlignment: MainAxisAlignment.center,
                children: <Widget>[
                  Text(
                    '${state.value}',
                    style: Theme.of(context).textTheme.headline4,
                  ),
                  Text(
                    'last updates: ${state.lastUpdated}',
                    style: Theme.of(context).textTheme.caption,
                  ),
                ],
              ),
            );
          },
        ),
      ),
      floatingActionButton: FloatingActionButton(
        child: Icon(Icons.refresh),
        heroTag: 'fab2',
        onPressed: () => context.cubit<TestCubit>().fetch(),
      ),
    );
  }
}

class TestCubit extends Cubit<Data> {
  TestCubit(this._repository) : super(Data('waiting'));

  final Repository _repository;

  Future<void> fetch() async {
    final data = await _repository.getData();
    emit(data);
  }
}

class Repository {
  Data _cachedData;

  Future<Data> getData() async {
    if (_cachedData != null) return _cachedData = _cachedData.copyWith();
    await Future<void>.delayed(const Duration(seconds: 1));
    return _cachedData = Data('it works');
  }
}

class Data {
  Data(this.value, [DateTime lastUpdated])
      : lastUpdated = lastUpdated ?? DateTime.now();

  final String value;
  final DateTime lastUpdated;

  Data copyWith({String value}) {
    return Data(value ?? this.value, DateTime.now());
  }
}

@saschaernst
Copy link
Author

You are using the same cubit instance for both pages, right? That will obviously work. My problem is with multiple cubit instances (one per page, no matter if of the same type or not) using the same repo. The second cubit instance will be in trouble

@felangel felangel added bug Something isn't working and removed question Further information is requested waiting for response Waiting for follow up labels Jul 5, 2020
@felangel felangel moved this from To do to In progress in cubit Jul 5, 2020
cubit automation moved this from In progress to Done Jul 5, 2020
@felangel felangel reopened this Jul 5, 2020
cubit automation moved this from Done to In progress Jul 5, 2020
@felangel
Copy link
Owner

felangel commented Jul 5, 2020

@saschaernst I found the issue and am working on pushing out a fix 👍
Should have this patched sometime today (taking a break for dinner) haha.

@felangel felangel mentioned this issue Jul 6, 2020
7 tasks
@felangel
Copy link
Owner

felangel commented Jul 6, 2020

@saschaernst I've released cubit 0.2.0-dev.1 and flutter_cubit 0.2.0-dev.1 which include the fix 🎉.
Can you please try updating and let me know if the issue has been resolved? Thanks 🙏

@felangel felangel added the waiting for response Waiting for follow up label Jul 6, 2020
@saschaernst
Copy link
Author

Works for me, you are my hero!

@felangel
Copy link
Owner

felangel commented Jul 6, 2020

Awesome! Thanks so much for reporting this and providing detailed reproduction steps, I really appreciate it 🙏

@felangel felangel closed this as completed Jul 6, 2020
cubit automation moved this from In progress to Done Jul 6, 2020
@saschaernst
Copy link
Author

It was an honour to help you out. When can we expect the final release of 0.2.0?

@felangel
Copy link
Owner

felangel commented Jul 6, 2020

There are some other things I need to sort out for 0.2.0 but hopefully by next week 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working waiting for response Waiting for follow up
Projects
No open projects
cubit
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants