TextEditingController.text docs miss a reason

[feinstein]

TextEditingController.text docs say it shouldn't be updated in a build method, but it doesn't explain why, it just says the listeners will be called. Is this for performance reasons, so the listeners won't lock the build method? Or are there any other issues as well?

I think the docs should include a more explained reason on the side-effects, so devs could evaluate more the impacts of their code and their decisions, or facilitate fixing bugs.

[gaaclarke]

https://github.com/flutter/flutter/blob/78951b0c01b7f5abc072974981d62830709dd9f1/packages/flutter/lib/src/widgets/editable_text.dart#L137

I think setting the property instead of using the constructor parameter would look very non-idiomatic. It sounds like they did attempt to explain why someone shouldn't do it. Since you found it confusing maybe you can suggest a fix @feinstein?

[feinstein]

@gaaclarke the text property in TextEditingController is for changing the text on the fly, like in a button. In this case, we don't want to rebuild a new TextEditingController, so the constructor parameter isn't helpful and we need a separate property for this.

I can't suggest an fix as I don't know why they suggested this. For example, if the recommendation is only about performance issues, then it should say something as :

.... as we can't know how many listeners are there and since they will be called synchronously, they can hang the build process leading to decreased performance and visible lag in animations.

But if the reason is something else entirely, like calling internal methods of the framework that shouldn't be called inside the build method, then I don't know, and only they can explain the reason.

[gaaclarke]

the text property in TextEditingController is for changing the text on the fly, like in a button. In this case, we don't want to rebuild a new TextEditingController

I think there is some confusion about Reative Programming. When you change a value you rebuild the widgets that rely on that value. Check out the default flutter project that gets created when you run flutter create foobar. The code looks like this:

@override
  Widget build(BuildContext context) {
    // This method is rerun every time setState is called, for instance as done
    // by the _incrementCounter method above.
    //
    // The Flutter framework has been optimized to make rerunning build methods
    // fast, so that you can just rebuild anything that needs updating rather
    // than having to individually change instances of widgets.
    return Scaffold(
      appBar: AppBar(
        // Here we take the value from the MyHomePage object that was created by
        // the App.build method, and use it to set our appbar title.
        title: Text(widget.title),
      ),
      body: Center(
        // Center is a layout widget. It takes a single child and positions it
        // in the middle of the parent.
        child: Column(
          // Column is also a layout widget. It takes a list of children and
          // arranges them vertically. By default, it sizes itself to fit its
          // children horizontally, and tries to be as tall as its parent.
          //
          // Invoke "debug painting" (press "p" in the console, choose the
          // "Toggle Debug Paint" action from the Flutter Inspector in Android
          // Studio, or the "Toggle Debug Paint" command in Visual Studio Code)
          // to see the wireframe for each widget.
          //
          // Column has various properties to control how it sizes itself and
          // how it positions its children. Here we use mainAxisAlignment to
          // center the children vertically; the main axis here is the vertical
          // axis because Columns are vertical (the cross axis would be
          // horizontal).
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text(
              'You have pushed the button this many times:',
            ),
            Text(
              '$_counter',
              style: Theme.of(context).textTheme.display1,
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: _incrementCounter,
        tooltip: 'Increment',
        child: Icon(Icons.add),
      ), // This trailing comma makes auto-formatting nicer for build methods.
    );

Notice how _counter is used in the Text widget and how it is getting set. That is the correct usage of the paradigm.

[feinstein]

That's correct for a StatelessWidget but with StatefulWidgets things aren't so simple, as your state can be defined in multiple places, callbacks, that's why the Flutter framework has several controllers, like TextEditingController, AnimationController, PageController, as explained here.

What you are implying is that the Flutter framework creators are wrong in exposing the text property, and not only this property, but all the other ones from several other controllers. This controller pattern is common in Flutter in general and is very useful for streamlining reactive programming, otherwise you will have to create several variables and update them in callbacks, which gets messy very quickly.

[feinstein]

@Hixie could you clarify this please?

[pedromassangocode]

Still valid since the actual documentation does not provide more details about the side effects of updating the text property during build phase.

https://api.flutter.dev/flutter/widgets/TextEditingController/text.html

[Hixie]

The reason to not set text during build is what the docs say -- it would cause listeners to fire, and you don't want listeners during during build because they might call setState, or do other things, that would invalidate the current build. In general, the build phase should have no business logic at all -- it should have no side effects, the build method is just a function that maps current state to widgets.

[feinstein]

@Hixie this is a very general recommendation for Flutter that I see often, is it written on any docs so we can link it for the text, for people (specially inexperienced) that want to learn more?

This way we keep the docs small and don't repeat it all over the place.

[Hixie]

Hmm, I can't think of anything where we say that. We really should. We should add it to the State.build and StatelessWidget.build API docs, for sure...

[feinstein]

Agreed. Do you want me to open an issue about it, or do we track it here?

[Hixie]

We can fix it as part of this issue.

[huycozy]

This issue is reproducible on the latest stable and master channels.

https://github.com/flutter/flutter/blob/78951b0c01b7f5abc072974981d62830709dd9f1/packages/flutter/lib/src/widgets/editable_text.dart#L137-L140

https://api.flutter.dev/flutter/widgets/TextEditingController/text.html