DropZoneView does not respect new values for the callbacks fields like `onDropFiles`

[rafalbednarczuk]

Hi!
The issue is that if I want to change DropZoneView's callbacks like onDropFiles by updating the widget tree with a new instance of DropZoneView with new callbacks, then it doesn't update its callbacks, instead it's using the callbacks of the first DropZoneView widget ignoring updates.

Environment:

Running on flutter web, chrome, macOS
Flutter 3.24.0 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 80c2e84975 (8 months ago) • 2024-07-30 23:06:49 +0700
Engine • revision b8800d88be
Tools • Dart 3.5.0 • DevTools 2.37.2

I made reproducible sample, where each build currentNumber is changed, but onDropFiles callback still prints the firstNumber as current number. So first callback value is used and then newer updated again.
The correct behavior, would be to remove previous callback and use the new one instead.

import 'dart:math' show Random;

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

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

class MyApp extends StatefulWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  String message1 = 'Drop something here';
  int? firstNumber;

  @override
  Widget build(BuildContext context) {
    final currentNumber = Random().nextInt(1000000);
    print("number:$currentNumber");
    firstNumber ??= currentNumber;
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(
          title: Text("title"),
        ),
        body: Column(
          children: [
            OutlinedButton(
                onPressed: () {
                  setState(() {});
                },
                child: Text("Rebuild")),
            Expanded(
              child: Stack(
                children: [
                  DropzoneView(
                    onDropFiles: (files) {
                      print(
                          'Zone 1:$currentNumber:$firstNumber drop multiple: $files');
                    },
                  ),
                  Center(child: Text(message1)),
                ],
              ),
            ),
          ],
        ),
      ),
    );
  }
}

[deakjahn]

I'm not entirely sure. I mean, we do normally use didUpdateWidget() to change something in the state when the widget changes but the callbacks are not in the state but in the widget itself, so they get recreated with the possible new values all the time...

[rafalbednarczuk]

so they get recreated with the possible new values all the time...

The problem is that a new widget with new callbacks isn't respected, the previous callbacks are still used despite changing the widget to a new one that has different callbacks.
When widget is changed, the callbacks that are inside the widget should also be used instead of callbacks from the previous widget.

[deakjahn]

This isn't the plugin, this is Dart. You create a closure for your callback and that will remain unmodified throughout the life of the app. The fact that you change your currentNumber is no longer reflected in the closure, it will always use the value that was current when the closure was created. So, you don't really change your callbacks at all. To see that, put a

print("onDropFiles ${widget.onDropFiles}");

as the first line of _DropzoneViewState.build() and you'll see that it never gets called repeatedly, just the very first time with the closure.

You should put the variable inside your state and modify it using the standard setState() mechanism. When you assign a new value to your currentNumber and do it inside setState(), this will force a new build() to be called automatically, and that will in turn pick up your newer, current value.

[rafalbednarczuk]

... So, you don't really change your callbacks at all. To see that, put a

print("onDropFiles ${widget.onDropFiles}");

as the first line of _DropzoneViewState.build() and you'll see that it never gets called repeatedly, just the very first time with the closure.

I did put as the first line of _DropzoneViewState.build():

    print("onDropFiles:${widget.onDropFiles.hashCode}:${widget.onDropFiles}");

and then I ran code from my first message snippet.

In the snippet, there is a button, that calls setState that does rebuild.

Each rebuild, the line from _DropzoneViewState.build() is called. You can see from the log that widget.onDropFiles is changed, but plugin doesn't respect the change and uses the first callback:

number:397101
onDropFiles:119620910:Closure: (List<DropzoneFileInterface>?) => void from: files => {
                        core.print("Zone 1:" + dart.strSafe(currentNumber) + ":" + dart.str(this.firstNumber) + " drop multiple: " + dart.str(files));
                      }
Zone 1:397101:397101 drop multiple: [Instance of 'DropzoneFileWeb']
//[Button click, REBUILD HAPPENS changing number, creating new callback and forcing _DropzoneViewState.build]
number:947898
onDropFiles:467972827:Closure: (List<DropzoneFileInterface>?) => void from: files => {
                        core.print("Zone 1:" + dart.strSafe(currentNumber) + ":" + dart.str(this.firstNumber) + " drop multiple: " + dart.str(files));
                      }
Zone 1:397101:397101 drop multiple: [Instance of 'DropzoneFileWeb']

[deakjahn]

Again... You use a local variable that's captured by the closure. Do it like this:

import 'dart:math' show Random;

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

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

class MyApp extends StatefulWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  String message1 = 'Drop something here';
  late int firstNumber;
  late int currentNumber;

  @override
  void initState() {
    super.initState();

    currentNumber = Random().nextInt(1000000);
    firstNumber = currentNumber;
  }

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(
          title: Text("title"),
        ),
        body: Column(
          children: [
            OutlinedButton(
                onPressed: () {
                  setState(() {
                    currentNumber = Random().nextInt(1000000);
                  });
                },
                child: Text("Rebuild")),
            Expanded(
              child: Stack(
                children: [
                  DropzoneView(
                    onDropFiles: (files) {
                      print('Zone 1:$currentNumber:$firstNumber drop multiple: $files');
                    },
                  ),
                  Center(child: Text(message1)),
                ],
              ),
            ),
          ],
        ),
      ),
    );
  }
}

Now the result will be:

Zone 1:838452:838452 drop multiple: [Instance of 'DropzoneFileWeb']
Zone 1:927287:838452 drop multiple: [Instance of 'DropzoneFileWeb']
Zone 1:690330:838452 drop multiple: [Instance of 'DropzoneFileWeb']
Zone 1:644028:838452 drop multiple: [Instance of 'DropzoneFileWeb']

Besides, you don't even want your callback to be replaced actually. You have the same callback, the very same code inside, only that the code should use the current value of currentNumber. So, don't use a currentNumber variable local to build() but one that's in the state. If you use a local variable, it will be captured and will never change. Using a regular, state variable makes sure that while the function itself stays the same, it always gets the current value of whatever variable it uses.

I would even prefer to create a separate function and just call its tear-off in the DropzoneView constructor. This is mostly a question of personal taste, of course, I prefer to use separate functions for anything more complex that a real one-liner.

[rafalbednarczuk]

From your quote, this is an example of the bug I reported:

If you use a local variable, it will be captured and will never change.

An example of correct behavior would be my first snippet using OutlineButton instead of DropZonView and changing onPressed instead of onDropFiles:

import 'dart:math' show Random;

import 'package:flutter/material.dart';

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

class MyApp extends StatefulWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  int? firstNumber;

  @override
  Widget build(BuildContext context) {
    final currentNumber = Random().nextInt(1000000);
    print("number:$currentNumber");
    firstNumber ??= currentNumber;
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(
          title: Text("title"),
        ),
        body: Column(
          children: [
            OutlinedButton(
                onPressed: () {
                  setState(() {});
                },
                child: Text("Rebuild")),
            SizedBox(height: 32),
            OutlinedButton(
              onPressed: () {
                print('Zone 1:$currentNumber:$firstNumber onPressed');
              },
              child: Text("Press me"),
            ),
          ],
        ),
      ),
    );
  }
}

The log is

number:62605
Zone 1:62605:62605 onPressed
//[Button click, REBUILD HAPPENS changing number]
number:932672
Zone 1:932672:62605 onPressed

With OutlineButton, the callback is changed as expected. With DropZoneView the callback is not changed as expected.

You provided me workarounds to the bug, but I already know the workaround. The workaround is not optimal and bug-prone.

[deakjahn]

Sorry, I'm still not convinced. In my opinion, using a state variable, and most importantly, not using a local variable for state purposes in build() is not a workaround but the fundamental way Flutter is expected to work. Yes, I see your sample but if you look deeper into OutlinedButton, this is probably merely a happenstance, resulting from the fact that while the button is a stateful widget like anything else, it happens to build itself using an InkWell/InkResponse that is stateless. So, this doesn't look to me like a standard behavior that you can count on with Flutter widgets in general.

Also, I still don't understand why you attribute any importance to a callback being replaced or not. Clearly, in your scenario, you don't need the callback itself replaced, you simply need the callback to use the actual value of a variable external to it. Your callback, as a function, a piece of code, doesn't change at all between rebuilds, no matter what the widget does.

[rafalbednarczuk]

Yes, I see your sample but if you look deeper into OutlinedButton, this is probably merely a happenstance, resulting from the fact that while the button is a stateful widget like anything else, it happens to build itself using an InkWell/InkResponse that is stateless. So, this doesn't look to me like a standard behavior that you can count on with Flutter widgets in general.

The standard behavior in all the flutter foundation widgets is that a callback is not preserved from the first widget, it's not a happenstance.
InkResponse to build itself under the hood is using _InkResponseStateWidget, which is Stateful, it doesn't matter if it's stateless or stateful, the callback change is updated.
The fundament way Flutter is expected to work is that a widget can be changed and new widget configuration is applied, not ignored.
The current behavior of DropZoneView is counterintuitive.

The reproducible sample was provided so it's easy to show there is a bug, it's not my scenario.

[deakjahn]

OK, then let's join forces in investigating. You're right with _InkResponseState, I haven't gone deep enough, still, it also doesn't manually change callbacks in its didUpdateWidget() where such an operation would logically take place. Actually, I've never seen any widget — either stock or contributed that I ever looked into or modified for my own use — that does anything similar with callbacks. Generally, widgets that have controllers replace them in this function but nothing else (I have a few other contributed widgets as well, like crop_image, it does that, too, because it has a controller).

I can see that you also authored quite a few packages. Do you have some where you handled this case or could we identify how stock widgets you tried do so?

[rafalbednarczuk]

I think the easy way to fix the issue in DropZoneView is to

1. Add corresponding callbacks functions in _DropzoneViewState.
2. The callback functions from the state should just call DropzoneView widget callbacks.
3. Change how DropZoneViewController._create works. It should use State callbacks instead of widget callbacks.

So the fix is to create a workaround inside the DropZoneView, so then the workaround doesn't have to be applied outside the widget.

[deakjahn]

Wait a minute first. We have an internal controller but there's no controller change in didUpdateWidget() (actually, there's no such function at all). Like I mentioned from my other widget package:

https://github.com/deakjahn/crop_image/blob/ce4b44fcbfd0ce95943d0be374e114b2df5d4215/lib/src/crop_image.dart#L219-L228

There's something like that there because the user can provide a controller of their own. What if we just recreate the controller here? That would call _create() again. I'll try this first.

[deakjahn]

This wasn't enough, didUpdateWidget() is also needed. Check it out. I also have some other modifications in the commit (mostly reformatting, now that the Dart formatter finally abandoned the stupid 80-character line limit. :-)