Skip to content

PersistentBottomSheetController's type parameter is unused; should be removed #137424

@srawlins

Description

@srawlins

I could be mistaken, but PersistentBottomSheetController is currently generic, PersistentBottomSheetController<T>, and the type variable, T, provides no value; it is usually void, sometimes dynamic, but should just be removed. Here are the current declarations:

class PersistentBottomSheetController<T> extends ScaffoldFeatureController<_StandardBottomSheet, T> {
  const PersistentBottomSheetController._(...);

  // No uses of T in the declaration.
  // ...
}

class ScaffoldFeatureController<T extends Widget, U> {
  // ...
  final Completer<U> _completer;
  Future<U> get closed => _completer.future;
  // ...
}

A few points:

  • PersistentBottomSheetController has one constructor, which is private; all instantiations of this class are controlled from the library in which it is declared, lib/src/material/scaffold.dart.
  • PersistentBottomSheetController never references T in its declaration.
  • The only use for the type variable, U, on ScaffoldFeatureController, is for the private field, Completer<U> _completer, and the associated public getter, Future<U> get closed.
  • The one (one) place where PersistentBottomSheetController._() is called, is _buildBottomSheet. This function creates a Completer<T>, but then always completes that Completer with a value of null (completer.complete()). There is nothing in the public API which can affect what the Completer completes with; it is always null. So,
    • the Completer should really be a Completer<void>,
    • the returned PersistentBottomSheetController should be a PersistentBottomSheetController<void>, and the PersistentBottomSheetController class should extend ScaffoldFeatureController<_StandardBottomSheet, void>.

Because the constructor is private, and the type variable is unused (has no effect on any PersistentBottomSheetController instance), you'd hope it could be removed in a non-breaking change. But it's not quite so simple. Here is the public API which instantiates PersistentBottomSheetController:

  • PersistentBottomSheetController<T> showBottomSheet<T>(...)
  • PersistentBottomSheetController<T> Scaffold.showBottomSheet<T>(...)

These two functions are also generic, and their type variable, T, is only used for inference to return a PersistentBottomSheetController<T>, so each function's type variable is also unused. In a complete fix, the type variables would be removed from the PersistentBottomSheetController class, and the showBottomSheet and Scaffold.showBottomSheet functions, which would be breaking, for each position in user code that explicitly specifies a type variable.

If a non-breaking (or, less breaking) intermediate change would be desirable, these functions, and even the class, could remain generic, but then literally reference that type variable in 0 places; so the functions would still have a <T>, but return a PersistentBottomSheetController<void>. The class would still have a <T>, but would extend ScaffoldFeatureController<_StandardBottomSheet, void>.

Metadata

Metadata

Assignees

Labels

P2Important issues not at the top of the work listc: proposalA detailed proposal for a change to Flutterf: material designflutter/packages/flutter/material repository.frameworkflutter/packages/flutter repository. See also f: labels.team-designOwned by Design Languages team

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions