Skip to content

feat(ensemble_widget): manage shared controller bindings and improve listener cleanup - #2377

Merged
usmanvrtx merged 6 commits into
android_TV_implementationfrom
fix/unclosed-listeners-tv
Sep 29, 2026
Merged

usmanvrtx merged 6 commits into
android_TV_implementationfrom
fix/unclosed-listeners-tv

Conversation

@usmanvrtx

@usmanvrtx usmanvrtx commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Description

This PR fixes binding listener leaks and lost subscriptions when widgets are rebuilt, moved between scopes, or temporarily disposed and remounted.

It also ensures that child scopes do not tear down page-owned resources, while page disposal cancels and clears subscriptions, timers, and other page-owned references.

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

What Has Changed

  • Added shared binding ownership tracking for widgets that reuse a controller, so bindings are removed only after the last widget using that controller is gone.
  • Restore binding subscriptions when cached widgets are remounted, including custom widget inputs and widgets inside templates.
  • Remove stale listeners and registrations when widgets are replaced, controllers change, or bindings move to another scope.
  • Ensure child-scope disposal does not tear down resources shared by the page.
  • Clear page-owned binding listeners, timers, location listeners, and dialog references when the page scope is disposed.
  • Added regression tests for dialog and bottom-sheet cleanup, scope disposal, widget rebuilds, custom widget bindings, scope changes, and controller replacement.

How to Test

  1. From the repository root, run:
    melos bootstrap
  2. Run the focused regression tests:
    cd modules/ensemble
    flutter test test/ensemble_widget_rebind_test.dart
  3. Run the Ensemble module test suite:
    flutter test
  4. Verify that bindings continue updating after a widget is rebuilt or hidden and shown, and that closing dialogs or bottom sheets does not leave their listeners behind.

Screenshots / Videos

Not applicable.

Checklist

  • I have run flutter analyze and addressed any new warnings
  • I have run flutter test and all tests pass
  • I have tested my changes on the relevant platform(s)
  • I have updated documentation if needed
  • My changes do not introduce new warnings or errors

@usmanvrtx usmanvrtx self-assigned this Sep 28, 2026
@usmanvrtx
usmanvrtx force-pushed the fix/unclosed-listeners-tv branch from 9db9546 to 0789d5d Compare September 28, 2026 00:25
@usmanvrtx usmanvrtx changed the title feat(ensemble_widget): manage shared controller bindings and improve … feat(ensemble_widget): manage shared controller bindings and improve listener cleanup Sep 28, 2026
@usmanvrtx
usmanvrtx force-pushed the fix/unclosed-listeners-tv branch from 70420e5 to 9d0a655 Compare September 28, 2026 15:34

@sharjeelyunus sharjeelyunus left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description mentions test/ensemble_widget_rebind_test.dart, but that test file is not currently part of the diff.

At minimum I’d like tests covering:

  • two mounted widgets sharing one controller;
  • disposing one owner keeps the binding alive;
  • disposing the final owner removes the listener;
  • disposed/cached widget remount restores exactly one subscription;
  • controller replacement;
  • scope A → scope B movement;
  • two sibling child scopes sharing the same PageData;
  • repeated dialog/bottom-sheet open/close does not grow listenerMap;
  • page disposal clears registrations/owners/listeners.

Given this code sits in the core binding lifecycle, I’d treat those as required regression tests rather than relying on manual verification.

Comment thread modules/ensemble/lib/widget/custom_widget/custom_widget.dart
Comment thread modules/ensemble/lib/framework/scope.dart
Comment thread modules/ensemble/lib/framework/scope.dart
@usmanvrtx
usmanvrtx merged commit 481e112 into android_TV_implementation Sep 29, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants