Why don't we make _TabBarScrollController public but final?

I understand that the private _TabBarState needs to stay private, but why don't we include a private setter for it?

final class TabBarScrollController extends ScrollController {
  /// The state of the attached [TabBar], if any.
  ///
  /// Is null when not attached to a [TabBar].
  _TabBarState? _tabBarState;

  void _attachToTabBar(_TabBarState tabBar) {
     _tabBarState = tabBar;
  }

  @override
  ScrollPosition createScrollPosition(
    ScrollPhysics physics,
    ScrollContext context,
    ScrollPosition? oldPosition,
  ) {
    return _TabBarScrollPosition(
      physics: physics,
      context: context,
      oldPosition: oldPosition,
      tabBar: _tabBarState!,
    );
  }
}

Then we only need to call the attach method using a private helper method? (which handles creating an internal controller and attaches the this)

For example:

TabBarScrollController _setUpTabBarController() {
   final tabBarController = widget.tabBarController ?? TabBarScrollController();
   
   tabBarController._attachToTabBar(this);
   
   return tabBarController;
}

// ...
_scrollController ??= _setUpTabBarController()

Thanks for your review, @navaronbracke!
If we proceed with this approach:

1. how do we initialize a TabBarState to pass into the TabBarScrollController constructor? And we need to make TabBarState public too, for this purpose.
2. Even if this were feasible, I'm quite concerned about the integrity and encapsulation of TabBarState class since it's marked as a public class.
3. Since marking TabBarState as public, the cost of maintenance for it (new public API) may increase, as users may encounter unexpected issues with this change in their usage?

Also, looking back the original use case from OP at #123965 reported:"Exposing the Scroll Controller of a TabBar with isScrollable: true, will let developers apply custom behavior based on the scroll.", it means they only need offset from scrollController which is sufficient enough?

With these reasons, I think the callback approach is sufficient and safer.

1. It would never be a constructor parameter in my proposal.

For attaching a TabBarState, I had proposed using a new function, that is private to TabBarController only, so it handles this automatically.

  void _attachToTabBar(_TabBarState tabBar) {
     _tabBarState = tabBar;
  }

which is called right after the line that currently does this:

_scrollController ??= _TabBarScrollController(this);

It would then become:

_scrollController ??= _effectiveTabBarScrollController.._attach(this);

2. TabBarState stays private. We should not change this.
3. The only usage change is making TabBarScrollController's constructor public. Attaching to the private TabBarState is always going to be private API. Users will be able to use the public ScrollController API though (since the tab controller inherits it), which fixes this bug.

Thanks for your update!

It would never be a constructor parameter in my proposal.

I thought it'd be a constructor param as I saw your snippet code above, which contains widget.tabBarController:

final tabBarController = widget.tabBarController ?? TabBarScrollController();

Users will be able to use the public ScrollController API though (since the tab controller inherits it), which fixes this bug.

Could you please explain how users can access the exposed controller for their needs?

Also, would you mind submitting another pull request for your solution (which will include tests and sample codes too)? Then the team can consider which approach is better for this case. I think @Piinks is the best person to review this, just let's wait until the holidays are over.

Oh, widget.tabBarController would be a new constructor parameter for the TabBar widget (a nullable TabBarController), not a change to the TabBarController constructor.

I will make a new pull request with my proposal. I'll add you as reviewer as well.

@huycozy See #180389