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

FutureBuilder rebuilds unnecessarily when we long press on FAB button or appBar UI. #11426

Closed
cornelius-tan opened this issue Jul 27, 2017 · 34 comments
Assignees
Labels
f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.

Comments

@cornelius-tan
Copy link

We have two pages - Page 1(home route) and Page 2. Page 2 fetches text content from the Web and display it through a FutureBuilder widget. When we long press on the FAB button of Page 2 (to reveal tooltip), FutureBuilder refreshes and another network request is made for the same content.

Steps to Reproduce

  1. Run the code below.
  2. Click 'open page' to navigate to the Page 2. We simulate network request with Future.delayed().
  3. CircularProgessIndicator is shown for a while then the content is shown.
  4. Long press on the FAB button. FutureBuilder rebuilds!
    Now long press on the search icon on the appbar. FutureBuilder rebuilds again.

The bug only occurs when the FutureBuilder is not located on the home route. If we declare FutureBuilder on Page 1 instead of Page 2, the bug is not observed.

import 'dart:async';
import 'package:flutter/material.dart';

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

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return new MaterialApp(
      title: 'Debugging Futurebuilder',
      theme: new ThemeData(
        primarySwatch: Colors.blue,
      ),
      routes: <String, WidgetBuilder>{
        '/page2': (BuildContext context) => new Page2(),
      },
      home: new Page1(),
    );
  }
}

class Page1 extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return new Scaffold(
      appBar: new AppBar(
        title: new Text('Page 1'),
      ),
      body: new Center(
        child: new RaisedButton(
          child: new Text('Open page'),
          onPressed: () => Navigator.of(context).pushNamed('/page2'),
        )
      ),
    );
  }
}


class Page2 extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return new Scaffold(
      appBar: new AppBar(
        title: new Text('Page 2'),
        actions: <Widget>[
          new IconButton(
            icon: new Icon(Icons.search),
            tooltip: 'Search',
            onPressed: () {}  // does nothing
          )
        ],
      ),
      body: new FutureBuilder<String>(
        future: _simulateNetworkRequest(), // a Future<String> or null
        builder: (BuildContext context, AsyncSnapshot<String> snapshot) {
          switch (snapshot.connectionState) {
            case ConnectionState.none:
            case ConnectionState.waiting:
              return new Center(child: new CircularProgressIndicator());
            default:
              if (snapshot.hasError)
                return new Text('Error: ${snapshot.error}');
              else
                return new Center(child: new Text('Result: ${snapshot.data}'));
          }
        },
      ),
      floatingActionButton: new FloatingActionButton(
        child: new Icon(Icons.add),
        tooltip: 'Add',
        onPressed: () {}
      ),
    );
  }

  Future<String> _simulateNetworkRequest() {
    return new Future.delayed(const Duration(seconds: 2), () =>
      'A random text snippet from the Web');
  }
}

Flutter Doctor

[√] Flutter (on Microsoft Windows [Version 10.0.15063], locale en-US, channel master)
    • Flutter at C:\Users\tzm\Downloads\flutter_sdk
    • Framework revision 1d8705f3db (22 hours ago), 2017-07-26 11:44:50 -0700
    • Engine revision 3a12bc092d
    • Tools Dart version 1.25.0-dev.7.0

[√] Android toolchain - develop for Android devices (Android SDK 25.0.3)
    • Android SDK at C:\Users\tzm\AppData\Local\Android\sdk
    • Platform android-26, build-tools 25.0.3
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 1.8.0_112-release-b06)

[√] Android Studio (version 2.3)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Java version OpenJDK Runtime Environment (build 1.8.0_112-release-b06)

[√] IntelliJ IDEA Community Edition (version 2017.1)
    • Flutter plugin version 15.2
    • Dart plugin version 171.4424

[√] Connected devices
    • SM G7102 • 772e5ea4 • android-arm • Android 4.4.2 (API 19)
@cornelius-tan cornelius-tan changed the title FutureBuilder rebuilds unnecessarily when we long press on FAB and appBar UI. FutureBuilder rebuilds unnecessarily when we long press on FAB button or appBar UI. Jul 27, 2017
@cornelius-tan
Copy link
Author

cornelius-tan commented Jul 29, 2017

If I change Page2 FutureBuilder to its equivalent in StreamBuilder, the problem goes away.
Now press and hold the FAB button in Page2, the builder will not rebuild which is what we want. I guess the problem lies with the bundled async library because the Future fires for unknown reason. Further investigation needed.

class Page2 extends StatefulWidget {
  @override
  _Page2State createState() => new _Page2State();
}

class _Page2State extends State<Page2> {
  Stream<String> myStream;

  @override
  Widget build(BuildContext context) {
    return new Scaffold(
      appBar: new AppBar(
        title: new Text('Page 2'),
        actions: <Widget>[
          new IconButton(
            icon: new Icon(Icons.search),
            tooltip: 'Search',
            onPressed: () {}  // does nothing
          )
        ],
      ),
      body: new StreamBuilder<String>(
        stream: myStream, // a Future<String> or null
        builder: (BuildContext context, AsyncSnapshot<String> snapshot) {
          switch (snapshot.connectionState) {
            case ConnectionState.none:
            case ConnectionState.waiting:
              return new Center(child: new CircularProgressIndicator());
            default:
              if (snapshot.hasError)
                return new Text('Error: ${snapshot.error}');
              else
                return new Center(child: new Text('Result: ${snapshot.data}'));
          }
        },
      ),
      floatingActionButton: new FloatingActionButton(
        child: new Icon(Icons.add),
        tooltip: 'Add',
        onPressed: () {}
      ),
    );
  }

  Future<String> _simulateNetworkRequest() {
    print('Request made');
    return new Future.delayed(const Duration(seconds: 2), () =>
      'A random text snippet from the Web');
  }

  @override
  void initState() {
    super.initState();
    myStream = _simulateNetworkRequest()?.asStream();
  }

}

@Hixie Hixie added the f: routes Navigator, Router, and related APIs. label May 8, 2018
@Hixie Hixie added this to the Goals milestone May 8, 2018
@Hixie Hixie added the framework flutter/packages/flutter repository. See also f: labels. label May 8, 2018
@Hixie
Copy link
Contributor

Hixie commented May 8, 2018

This might be fixed with my work on making pages build less (not sure I've landed it yet), but I'll take a look at some point.

@putraxor
Copy link

putraxor commented Jun 23, 2018

this issue still persist

import 'dart:async';

import 'package:flutter/material.dart';

class Data {
  static Future<List<String>> mockFuture() async {
    await Future.delayed(Duration(milliseconds: 1500), () {});
    return [
      'https://swebtoon-phinf.pstatic.net/20180115_46/1515982322405V9H8X_JPEG/10_EC8DB8EB84A4EC9DBC_ipad.jpg',
      'https://swebtoon-phinf.pstatic.net/20151006_293/1444125151125BQylv_JPEG/_EB93BBEB80AF_EBA38CEABCA5_E29587EABCB1_EB9384EB84B0_ipa.jpg',
      'https://swebtoon-phinf.pstatic.net/20150608_183/1433732708022aySQw_JPEG/EC8DB8EB84A4EC9DBC_ipad.jpg',
      'https://pm1.narvii.com/5905/aab79b9fbf033cb116e1272460d9b900692e672b_hq.jpg',
      'https://pm1.narvii.com/5905/92822c13952a7c280b49b9507599aabe695b5933_hq.jpg',
      'https://pm1.narvii.com/5905/30ed9dd47001fbd89cef61e58b0121611ed30b69_hq.jpg',
      'https://pm1.narvii.com/5905/25c1ca23b78b9ca78aeb9d02fae841707fc4fb1c_hq.jpg',
      'https://pm1.narvii.com/5905/1c04748521e3ebfaabcd8c0cd8a1bc2344156d1b_hq.jpg',
      'https://pm1.narvii.com/5905/f87a64c47f11f4063e42dfb8a67db799b67f5478_hq.jpg',
    ];
  }
}

///
///image list
///
class NetImageList1 extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    print('NetImageList1 built!');
    return Scaffold(
      appBar: AppBar(title: Text('Thriller')),
      backgroundColor: Colors.grey,
      body: FutureBuilder(
        future: Data.mockFuture(),
        builder: (_, AsyncSnapshot<List<String>> snapshot) {
          if (snapshot.connectionState == ConnectionState.done) {
            final rows = snapshot.data.map((it) => ImageWidget(it)).toList();
            return ListView.builder(
              itemCount: rows.length,
              addAutomaticKeepAlives: false,
              itemBuilder: (_, i) => rows[i],
            );
          }
          return Center(child: CircularProgressIndicator());
        },
      ),
    );
  }
}

///
///image widget
///
class ImageWidget extends StatelessWidget {
  final String imageUrl;
  const ImageWidget(this.imageUrl);

  @override
  Widget build(BuildContext context) {
    print('Image widget built ${imageUrl.hashCode}');
    return FutureBuilder(
      future: mockLoading(),
      builder: (_, AsyncSnapshot<String> snapshot) {
        if (snapshot.connectionState == ConnectionState.done) {
          return Padding(
            padding: const EdgeInsets.all(4.0),
            child: InkWell(
              onTap: () => gotoDetail(context, imageUrl),
              child: Card(
                child: AspectRatio(
                    aspectRatio: 16 / 9, child: Image.network(imageUrl)),
              ),
            ),
          );
        }
        return AspectRatio(
            aspectRatio: 16 / 9,
            child: Container(
              color: Colors.white,
              child: Center(
                child: CircularProgressIndicator(),
              ),
            ));
      },
    );
  }

  gotoDetail(BuildContext context, String data) {
    Navigator.push(
        context, MaterialPageRoute(builder: (_) => NetImageDetail(data)));
  }

  Future<String> mockLoading() async {
    await Future.delayed(Duration(milliseconds: 250), () {});
    return imageUrl;
  }
}

///
///detail screen
///
class NetImageDetail extends StatelessWidget {
  final String imageUrl;
  NetImageDetail(this.imageUrl);
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text('Detail')),
      body: Center(child: Image.network(imageUrl)),
    );
  }
}

@AbdulRahmanAlHamali
Copy link

Hello,
I believe the problem here is that didUpdateWidget of the FutureBuilder state is being called every time a rebuild is issued. This function checks if the old future object is different from the new one, and if so, refires the FutureBuilder.

To get past this, we can call the Future somewhere other than in the build function. For example, in the initState, and save it in a member variable, and pass this variable to the FutureBuilder.

So instead of having:

FutureBuilder(
  future: someFunction(),
  ....

We should have:

initState() {
  super.initState();
  _future = SomeFunction();
}

and then

FutureBuilder(
  future: _future,
  ....

@sir-boformer
Copy link
Contributor

sir-boformer commented Aug 19, 2018

This is actually not a bug, but a feature.

The build method can be called many times per second, and there is no way to prevent that, so you should not make the assumption that build is only called once. That means your mockLoading() function is called many times, which is not your intention.

The build method should be pure, e.g. not cause any side-effects (except for subscriptions to inherited widgets). In a StatelessWidget, the output of the build method should only depend on:

  • Final constructor parameters of the widget itself
  • Data from inherited widgets (Flutter makes sure that build is called again when the data changes)

@AbdulRahmanAlHamali provided a correct solution to the problem.

@zoechi
Copy link
Contributor

zoechi commented Aug 20, 2018

@sir-boformer

should make the assumption that build is only called once

Is there a not missing after should?

@sir-boformer
Copy link
Contributor

Yep, corrected

@hyochan
Copy link

hyochan commented Aug 26, 2018

@sir-boformer Do you mean we shouldn't use FutureBuilder inside StatefulWidget ?

@sir-boformer
Copy link
Contributor

No. I said that build functions must be pure.

Actually using a FutureBuilder usually implies that the enclosing widget is stateful, because you have to store the future you are waiting for in a State object.

@hyochan
Copy link

hyochan commented Aug 26, 2018

@sir-boformer noob question but how to ensure the build function is pure? Could you show some examples?

@sir-boformer
Copy link
Contributor

sir-boformer commented Aug 27, 2018

Bad:

Color _randomColor;

Widget build(BuildContext context) {
  someService.loadData(); // not pure
  setState(() { // not pure
    this._randomColor = getRandomColor(); // not pure
  })
  return Container(
    color: this._randomColor,
    child: StreamBuilder(
      stream = this._createStream(), // not pure
      ...
    ),
  );
}

Good:

Color _randomColor;
Stream<...> _stream;
void initState() {
  super.initState();
  someService.loadData();
  this._randomColor = getRandomColor(); 
  this._stream = this._createStream();
}

Widget build(BuildContext context) { // pure
  return _buildStreamBuilder(); // this is ok 
}

Widget _buildStreamBuilder() { // pure
  // only reads variables and builds widget
  return Container(
    color: this._randomColor,
    child: StreamBuilder(
      stream: this._stream,
      ...
    ),
  );
}

@hyochan
Copy link

hyochan commented Aug 27, 2018

@sir-boformer Wow. Thank you for being a great tutor! So, there should be nothing that changes state inside StreamBuilder. This is the point right?

@sir-boformer
Copy link
Contributor

Not really. There is no "inside". When the build method is executed, all of the code inside of it is executed. That means you instantiate and return widgets (calling their constructors). You can read (but not write) fields of the State and the widget. Nothing else. There should be no side effects (like calling loadData()). The creation of a stream is a side effect.

Same with the builder function of StreamBuilder. That's also a build method. Basically every method or callback that takes a context and returns a widget should be free of side effects.

Just assume that the build method can be called at any time, multiple times per second, for unknown reasons. Do you want to call loadData() more than one time? Probably not.

@SkullEnemyX
Copy link

Hello,
I believe the problem here is that didUpdateWidget of the FutureBuilder state is being called every time a rebuild is issued. This function checks if the old future object is different from the new one, and if so, refires the FutureBuilder.

To get past this, we can call the Future somewhere other than in the build function. For example, in the initState, and save it in a member variable, and pass this variable to the FutureBuilder.

So instead of having:

FutureBuilder(
  future: someFunction(),
  ....

We should have:

initState() {
  super.initState();
  _future = SomeFunction();
}

and then

FutureBuilder(
  future: _future,
  ....

This is the answer I was looking for and it really works...thanks

@zoechi
Copy link
Contributor

zoechi commented Feb 19, 2019

I can not reproduce the initial example

[✓] Flutter (Channel master, v1.2.2-pre.21, on Mac OS X 10.14.3 18D42, locale en-AT)

Are you still seeing this?

@sir-boformer
Copy link
Contributor

@zoechi This is not a bug, just a misuse of FutureBuilder. I think it should be clarified in the docs that the future should never be created in a build method because it can be called many times, but that's it.

@zoechi
Copy link
Contributor

zoechi commented Feb 19, 2019

@sir-boformer

I see. What about

https://docs.flutter.io/flutter/widgets/FutureBuilder-class.html

The future must have been obtained earlier, e.g. during State.initState, State.didUpdateConfig, or State.didChangeDependencies. It must not be created during the State.build or StatelessWidget.build method call when constructing the FutureBuilder. If the future is created at the same time as the FutureBuilder, then every time the FutureBuilder's parent is rebuilt, the asynchronous task will be restarted.

@sir-boformer
Copy link
Contributor

yep, it was there all along. perfect!

@zoechi
Copy link
Contributor

zoechi commented Feb 20, 2019

Sounds like this can be closed then.
Please let me know if you disagree and I'll reopen.

@zoechi zoechi closed this as completed Feb 20, 2019
@mahmoudatef955
Copy link

What if the futureBuilder is in a pageView and the future depends on the position

@jerryzhoujw
Copy link

jerryzhoujw commented Aug 7, 2019

Yeah, maybe better to update the demo in that video too .
From

FutureBuilder(
   future: http.get('http://awesome.data'), 
   builder: _myBuilderFunction,
)

To

Future _myFuture = http.get('http://awesome.data');  
FutureBuilder(
   future: _myFuture, 
   builder: _myBuilderFunction,
)

@bestK
Copy link

bestK commented Sep 1, 2019

@frank06
Copy link

frank06 commented Dec 20, 2019

I think the summary of the problem is well laid out in that article, but I don't see why using a memoizer in combination with a StatefulWidget when you can already cache the value in its State.

Here's an example of how to fix this with Provider:

class MyWidget extends StatelessWidget {
  // Future<String> callAsyncFetch() => Future.delayed(Duration(seconds: 2), () => "hi");
  @override
  Widget build(BuildContext context) {
    // print('building widget');
    return FutureProvider<String>(
      create: (_) {
        // print('calling future');
        return callAsyncFetch();
      },
      child: Consumer<String>(
        builder: (_, value, __) => Text(value ?? 'Loading...'),
      ),
    );
  }
}

For those interested I wrote a full-blown article summarizing the problem and different approaches to solve it using StatefulWidget, provider and flutter_hooks. Hope it helps clarify this unfortunately very common issue

@erperejildo
Copy link

I tried moving the function outside as you guys suggested but it doesn't work, it's getting call all the time:

@override
  void initState() {
    super.initState();
    _future = authService.getUser();
  }

  @override
  Widget build(BuildContext context) {
    return FutureBuilder<FirebaseUser>(
      future: _future,
      builder: (context, AsyncSnapshot snapshot) {
        if (snapshot.connectionState == ConnectionState.done) {

@erperejildo
Copy link

@frank06 I had a look to your article but could you show what do you have inside FutureProvider? It's not clear to me

@frank06
Copy link

frank06 commented Jan 10, 2020

@erperejildo what do you mean "inside FutureProvider? There's a callAsyncFetch sample function (for which I even provided a sample implementation), that is all. Did you see the live example I did with Streams, a bit below? That does work. Can you try doing the same with your future (instead of a stream)? It looks almost exactly the same.

@erperejildo
Copy link

erperejildo commented Jan 10, 2020

Sorry @frank06, I thought it was a custom method inside a provider you had created. Just checked what it's exactly (https://pub.dev/documentation/provider/latest/provider/FutureProvider-class.html)

return StreamProvider<FirebaseUser>(
      create: (_) {
        print('calling future');
        return FirebaseAuth.instance.onAuthStateChanged;
      },
      child: Consumer<FirebaseUser>(
        builder: (_, value, __) => Text('Loaded' ?? 'Loading...'),
      ),
    );

That works fine but I still see the print every time I scroll on me list. Does it mean it is also being created every time?

Edit: when I log out it still showing Loaded

@frank06
Copy link

frank06 commented Jan 10, 2020

@erperejildo You should place the create further up the widget tree. I would try doing this with a StreamBuilder/FutureBuilder and understanding how it works before using Provider

@erperejildo
Copy link

Got you. I had my StreamBuilder before inside the ListView so you recommendation is having this outside?
Let me show you more graphically what I have:
Screenshot_1578670526

That list contains 2 different kind of sets: one that I load from a json file previously when the app is loading (to avoid a slow Hero transition to this page) and the one I get from Firebase. Both of them are as I said on the same scroll container.

If I move the create further up the widget tree, wouldn't be the default sets dependent on this StreamProvider?

@frank06
Copy link

frank06 commented Jan 10, 2020

@erperejildo Ok let me have a look later. Were you able to make it work with StreamBuilder? For StreamProvider, I'm pretty sure you have to use the .value constructor there instead of create. See the docs

If you already have an object instance and want to expose it, you should use the .value constructor of a provider.

@erperejildo
Copy link

with StreamBuilder and StreamProvider my widget works fine. The only thing is that, when I scroll or when I move between pages this will call again and I only want to trigger this function when I log in/out.

I tried with:

return StreamProvider<FirebaseUser>.value(
      value: FirebaseAuth.instance.onAuthStateChanged,
      child: Consumer<FirebaseUser>(
        builder: (_, value, __) {
          print('triggered');
          return Text('Loaded' ?? 'Loading...');
        },
      ),
    );

but the behaviour is exactly the same: I see the print when I scroll to the widget.

I'm sure this is what you mentioned, something more related with the way I put my widget on tree, but tbh not sure how should I move it without breaking the global scroll.

@frank06
Copy link

frank06 commented Jan 10, 2020

@erperejildo Sorry I can't see what you are trying to do. There's no problem in using StreamBuilders inside a ListView. Builders/build are designed to be called a million times. You can also register the provider value up in the tree and consume it further down, but it depends on what you want to do. Can you send me an e-mail with a bit more of your widget tree? And I'll help you out there, so we don't pollute this thread. E-mail is on my github profile.

@erperejildo
Copy link

erperejildo commented Jan 13, 2020

Thanks @frank06 but I didn't want to take more of your time so I've been investigating a little bit more creating a new fresh and simpler app just with this. I'll show what I've done in case I can help someone else.

First of all I worked on the cache for my provider creating this widget:

class FutureTestPage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            RaisedButton(
              child: Text('Load'),
              onPressed: () {
                Provider.of<Game>(context, listen: false).anotherLoad(); // in case we want to force an update
              },
            ),
            FutureBuilder<String>(
              future:
                  Provider.of<Game>(context, listen: false).loadResult(),
              builder: (BuildContext context, AsyncSnapshot<String> snapshot) {
                if (snapshot.connectionState == ConnectionState.waiting) {
                  log("you shouldn't see this loading...");
                  return CircularProgressIndicator();
                } else if (snapshot.connectionState == ConnectionState.done) {
                  return Text(
                      'Data: ${Provider.of<Game>(context).result}');
                } else {
                  return Text('Not loaded');
                }
              },
            ),
          ],
        ),
      ),
    );
  }
}

This is using this global provider:

class Game extends ChangeNotifier {
  String result = '';

  Game();

  Future<String> loadResult() async {
    if (this.result != '') {
      return result;
    }
    await http.get('https://jsonplaceholder.typicode.com/posts');
    this.result = 'RESULT from provider'; // we can return the response but just for an easier widget
  }
}

The provider returns a http response which is cached: first time we enter on the page it loads and it saves the value on result. If we leave this page and we enter again we won't load anything.

(Of course this works for responses we don't want to listen to)

We can also move the loadResult() to an init function that we can call from the main page for example, and we could avoid the unnecessary loading when entering on our page.

you shouldn't see this loading... message was one of the part confusing me. This was triggering all the time so I thought that it was being loaded again, but it's not. It's shown all the time.
For example, to see this more clear we can have this:
await Future.delayed(Duration(seconds: 5));
instead of this:
await http.get('https://jsonplaceholder.typicode.com/posts');

First time we'll see the 5 wait and the loading message, and second one, we'll see the message as well, but the value will be there already on the screen.

This answers my question asking why is the FutureBuilder loading all the time... but not completely.

I recorded this situation where I have 2 StreamBuilders. Both are the same (just returning a different layout).
As you can see, when I scroll to top again, one of them (not both, I don't know why) is reloading:
https://youtu.be/ScSmp0d1BOY

This is the widget showing these elements of the scroll:

Widget build(BuildContext context) {
    return SliverList(
      delegate: SliverChildListDelegate(
        [
          _MySetsLists(color), // the copy
          _MyFirebaseUser(), // the original
          _CreateSetButton(),
          Divider(),
        ],
      ),
    );
  }

I realised that the one loading again is the one on top. If I reorder them it will display the other loading message.

This part is what I don't really understand

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

No branches or pull requests