Calling "Logic.write()" during "initState()" leads to error

[Delgan]

Hi again!

This is a continuation of the discussion that took place in #2.

I put back here the offending code:

final fetchedCountRef = StateRef(0);

final dataFetcherRef = LogicRef((scope) => DataFetcher(scope));

class DataFetcher with Logic {
  const DataFetcher(this.scope);

  @override
  final Scope scope;

  int fetch(int num) {
    update<int>(fetchedCountRef, (count) => count + 1);
    return num * 2;
  }
}

class MyData extends StatefulWidget {
  final int num;

  const MyData(this.num);

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

class _MyDataState extends State<MyData> {
  int data;

  @override
  void initState() {
    super.initState();
    data = context.use(dataFetcherRef).fetch(widget.num);
  }

  @override
  Widget build(BuildContext context) => Text("Fetched data: $data");
}

Updating fetchedCountRef forces a rebuild, but this is not possible during initState() and raises an exception.

════════ Exception caught by widgets library ════════
The following assertion was thrown building _BodyBuilder:
setState() or markNeedsBuild() called during build.

This BinderScope widget cannot be marked as needing to build because the framework is already in the process of building widgets. A widget can be marked as needing to be built during the build phase only if one of its ancestors is currently building. This exception is allowed because the framework builds parent widgets before children, which means a dirty descendant will always be built. Otherwise, the framework might not visit this widget during this build phase.

I have (yet another) question: the documentation about context.use() states that it "cannot be called while building a widget". It's a little bit ambiguous for me: does it includes the initState() phase too?

From what I see in the source examples, you are systematically calling addPostFrameCallback() while using a Logic from initState(). Also, not using addPostFrameCallback() can cause subtle errors as reported by this ticket. Should context.use() be forbidden inside initState()?

Also, in #2 I said that as a workaround I would probably call Future.microtask(() => update(...)); in the Logic but finally I'm discarding this solution. 😄
It can lead to subtle bugs if another logic doesn't expect the change to be asynchronous. In addition, I prefer for the the logic to be decoupled from the view as much as possible.

So, I think I'm back to the FutureBuilder() solution that I initially used, but storing the Future as you suggested.

class _MyDataState extends State<MyData> {
  Future<int> data;

  @override
  void initState() {
    super.initState();
    data = Future.microtask(() => context.use(dataFetcherRef).fetch(widget.num));
  }

  @override
  Widget build(BuildContext context) => FutureBuilder(
        future: data,
        builder: (context, snapshot) => Text("Fetched data: ${snapshot.data ?? '?'}"),
      );
}

It's a bit inconvenient as it requires handling invalid data and forces the widget to be immediately rebuilt. However that's the solution I prefer. Additionally, I will refrain myself from directly using context.use() in any initState() method as I'm afraid of inadvertently breaking a widget while modifying a Logic method.

I don't know if other ideas will come to your mind. Sadly, it seems not possible to "fix" this internally without changing the binder api. It is understandable that you prefer not to extend the api for this particular use case. I have been thinking to a possible workaround, I wonder if it can be implemented as an extension. 🤔

data = context.borrow(dataFetcherRef, (ref) => ref.fetch(widget.num));

The idea would be to allow borrow() only inside initState(). The callback would be executed immediately, but first the scope would be somehow "marked" so that each call to setState() would be automatically postponed using addPostFrameCallback(). Then the scope would return to normal state.

In any case, this is as far as my knowledge goes. Thank you again for having looked into my problem! Hopefully, an elegant solution will eventually be found.

[letsar]

Yes I think I should prevent the use of Logic.use synchronously in initState but I have no way to tell (as I know) if we are in initState 😞 . To me, we should always delay the call to a method of the logic that mutate the state, that's why I create a widget for doing this in some examples.

Yes you're right, mutating the state asynchronously in a logic can lead to hard-to-find bugs.

Instead of the FutureBuilder, you can also call setState((){}) at the end of the future and making this widget generic (but you may already have done the latter with StateHolder<T>).

Concerning the borrow, I cannot prevent it to be called only in initState sadly.
There is maybe a solution for your use case, but I'm generally better to find solutions when I'm confronted myself to the issue. I don't despair that I will find an elegant solution for this, but for the moment there are no "Eureka"!

Thank your for your feedback 🙂

[Delgan]

Oh, yeah thanks for the suggestion to simply use setState() instead of FutureBuilder. I'm so used to ready-made widgets that I forget the basics. :D

Regarding the initState() detection, can't we use owner.debugBuilding?

I made a test with borrow(). Unfortunately, I don't think I can implement it as an extension, so I put the diff here for reference:

diff --git a/packages/binder/lib/src/binder_scope.dart b/packages/binder/lib/src/binder_scope.dart
index c409936..b55a567 100644
--- a/packages/binder/lib/src/binder_scope.dart
+++ b/packages/binder/lib/src/binder_scope.dart
@@ -56,6 +56,7 @@ class BinderScopeState extends State<BinderScope>
   final Set<BinderKey> readOnlyKeys = <BinderKey>{};
   final Set<BinderKey> writtenKeys = <BinderKey>{};
   bool clearScheduled = false;
+  bool borrowed = false;
 
   @override
   BinderScopeState parent;
@@ -157,9 +158,14 @@ class BinderScopeState extends State<BinderScope>
     if (isOwner(ref.key)) {
       void applyNewState() {
         readOnlyKeys.remove(ref.key);
-        setState(() {
+        if (borrowed) {
           states[ref.key] = state;
-        });
+          SchedulerBinding.instance.addPostFrameCallback((_) => setState(() {}));
+        } else {
+          setState(() {
+            states[ref.key] = state;
+          });
+        }
       }
 
       final effectiveObservers = [...observers, ...widget.observers];
diff --git a/packages/binder/lib/src/build_context_extensions.dart b/packages/binder/lib/src/build_context_extensions.dart
index 29e0af5..4a101ad 100644
--- a/packages/binder/lib/src/build_context_extensions.dart
+++ b/packages/binder/lib/src/build_context_extensions.dart
@@ -29,9 +29,19 @@ extension BinderBuildContextExtensions on BuildContext {
   ///
   /// Cannot be called while building a widget.
   T use<T>(LogicRef<T> ref) {
-    assert(!debugDoingBuild, 'Cannot call use() while building a widget.');
+    assert(!debugDoingBuild && !owner.debugBuilding, 'Cannot call use() while building a widget.');
     return readScope().use(ref);
   }
+
+  T borrow<T, U>(LogicRef<U> ref, T Function(U) callback) {
+    assert(!debugDoingBuild && owner.debugBuilding, 'Cannot call borrow() outside of initState().');
+    final scope = readScope();
+    final logic = scope.use(ref);
+    scope.borrowed = true;
+    final result = callback(logic);
+    scope.borrowed = false;
+    return result;
+  }
 }
 
 extension BinderBuildContextInternalExtensions on BuildContext {
diff --git a/packages/binder/lib/src/core.dart b/packages/binder/lib/src/core.dart
index f953369..996b1e9 100644
--- a/packages/binder/lib/src/core.dart
+++ b/packages/binder/lib/src/core.dart
@@ -70,6 +70,8 @@ abstract class Scope {
   /// Re-executes a previously canceled write.
   /// {@endtemplate}
   void redo();
+
+  void set borrowed(bool borrowed);
 }
 
 /// A part of the app state that can be watched.

Anyway, I don't know if I can say that, but "hopefully" one day you will be faced with this problem to better address it. 😁

Thanks for your time. 👍

[letsar]

Oh I didn't knew owner.debugBuilding, thanks!! Yes It seems it could work with this :-).
I will see what I can do to improve the API. I'm not sure about borrow though. English is not my native language, and I thus don't see the link between borrow and what the api will do, i.e. postpone the update to widgets. Can you tell me why you choose borrow?

[Delgan]

Well, English is not my native language either, that surely explains the trouble... 🙂

I haven't given it much thought. It was mostly for proof of concept. You have a better overview and you could definitely come up with a more appropriate name.

I just looked for something that resonated with context.use(). However, the widget is not usually allowed to directly access the Logic. Instead, the widget should "borrow" it temporarily for the time of the callback closure.

As a novice user, I don't really care about postponing the updates. I mainly care about being able to call Logic methods in initState().
The need to defer the setState() is a technical detail, not my goal.
Actually, most of the time, nothing will be delayed. We just want to protect ourselves from the special case involving write().

[letsar]

Oh I thought you were, since your comments are very well written!
Yes you're right, it was my technical view of the package which led me to think that postpone was the intention, but no, it's just a consequence.
Ok I understand why you choose borrow now!