adar2378 / pin_code_fields

A flutter package which will help you to generate pin code fields with beautiful design and animations. Can be useful for OTP or pin code inputs 🤓🤓
https://pub.dev/packages/pin_code_fields
MIT License
690 stars 336 forks source link

onChanged called multiple times with autoDisposeControllers: false #322

Closed jakobleck closed 1 year ago

jakobleck commented 1 year ago

Using the PinCodeTextField with an TextEditingController that shall be kept alive, i.e. setting autoDisposeControllers: false, it happens that when the PinCodeTextField is disposed and built again (say you navigate to the page with pin code field and back several times), afterwards the onChanged callback is called multiple times.

The fix is a one-liner, a removeListener for the TextEditingController is missing in PinCodeTextField's dispose. I'll open a PR.

jakobleck commented 1 year ago

Another problem that the PR would fix: Using the TextEditingController after the widget has been disposed, for example setting text, can result in an Exception:

======== Exception caught by foundation library ====================================================
The following assertion was thrown while dispatching notifications for TextEditingController:
setState() called after dispose(): _PinCodeTextFieldState#c9646(lifecycle state: defunct, not mounted, tickers: tracking 0 tickers)

This error happens if you call setState() on a State object for a widget that no longer appears in the widget tree (e.g., whose parent widget no longer includes the widget in its build). This error can occur when code calls setState() from a timer or an animation callback.

The preferred solution is to cancel the timer or stop listening to the animation in the dispose() callback. Another solution is to check the "mounted" property of this object before calling setState() to ensure the object is still in the tree.
This error might indicate a memory leak if setState() is being called because another object is retaining a reference to this State object after it has been removed from the tree. To avoid memory leaks, consider breaking the reference to this object during dispose().

When the exception was thrown, this was the stack: 
#0      State.setState.<anonymous closure> (package:flutter/src/widgets/framework.dart:1078:9)
#1      State.setState (package:flutter/src/widgets/framework.dart:1113:6)
#2      _PinCodeTextFieldState._assignController.<anonymous closure> (package:pin_code_fields/src/pin_code_fields.dart:444:9)
#3      ChangeNotifier.notifyListeners (package:flutter/src/foundation/change_notifier.dart:351:24)
#4      ValueNotifier.value= (package:flutter/src/foundation/change_notifier.dart:456:5)
#5      TextEditingController.value= (package:flutter/src/widgets/editable_text.dart:156:11)
#6      TextEditingController.text= (package:flutter/src/widgets/editable_text.dart:141:5)
...
The TextEditingController sending notification was: TextEditingController#6b93d(TextEditingValue(text: ┤├, selection: TextSelection.invalid, composing: TextRange(start: -1, end: -1)))
====================================================================================================
subzero911 commented 1 year ago

I've encountered this problem too. It seems to be already fixed by this PR: https://github.com/adar2378/pin_code_fields/pull/303 (mounted checking before setState). But it's not on pub.dev yet. I use a git dependency from master branch.

jakobleck commented 1 year ago

Hi @subzero911 , thanks for your comment. I would say you must have encountered a different problem: I have written a MWE to test the fix I'm making in this PR, and using the current master branch the error still exists. If you're interested, please let me know, I can upload the example app.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jakobleck commented 1 year ago

This is not stale, my PR is still awaiting merge. Alternatively PR #327 which does contain #323...

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.