Skip to content

Deprecate shrinkWrap and replace with API that is clearer about performance trade-offs #130360

Open
@Piinks

Description

@Piinks

Problem

When shrinkWrap is used for scrolling widgets, developers are often unaware of what it actually means, a loss of performance due to the entire contents of the scrollable being evaluated.

We have been working on bettering the experience here through the Scroll Performance Pitfalls Project for example:

There are a handful of times when shrinkWrap is appropriate, but most of the time users do not need to use it and cost themselves performance issues. It is very easy to just toggle the shrinkWrap flag, and either forget to come back and toggle it off (often the case when fleshing out the UI and the user gets the unbounded error) or revisit it to refactor for efficient scrolling. In some cases, it can be very difficult to refactor if additional UI has been built around the shrinkWrapped viewport (see the news toolkit PRs above). The internet is also working against us here, as there are multiple instances of shrinkWrap on StackOverflow and elsewhere as a recommended solution.


Proposal

For these reasons, we believe shrinkWrap should be deprecated and replaced with an API that is much less obscure as far as what decision the user is making.

For example:

// Old API
ListView(
  shrinkWrap: true, // This would be deprecated and ultimately removed
  children: myChildrenList,
);

// New API
NonLazyListView(
  children: myChildrenList,
);

// * Name subject to better minds' ideas :)

This code could be relatively simple, since ListView, GridView and CustomScrollView are already subclasses of ScrollView, the new classes for these could just explicitly set the shrinkWrap property of the super class rather than exposing it as a constructor parameter.


Communication & Migration

The biggest effort here will be communication and migration. There should be 4 main cases:

  1. In about half of the cases I have found, shrinkWrap can actually be removed. This often happens when an unbounded error is thrown, the user sets the flag, but then later the scroll view ends up being constrained and the flag is no longer necessary.

  2. The developer should refactor just wrapping an Expanded widget around the ScrollView widget. This is commonly seen when a ListView (or other) is placed inside of a Column.

  3. The developer should refactor to use a CustomScrollView & slivers instead of ListView (or other).
    a. Update some of our ListView samples with CustomScrollViews website#8435 - Refactoring away from shrinkWrap via the CustomScrollView option will be easier if we have more examples of CustomScrollView for our developers
    b. Add library of slivers to the website website#8433

  4. The developer should refactor to use a SingleChildScrollView if there is only one or very few children, it (basically) operates the same way as shrinkWrap.

  5. If it is a safe and relevant use of shrinkWrap, the developer can just switch to the new shrinkWrapping class by renaming the scrolling widget that they used to set shrinkWrap: true on.
    a. example: ListView -> NonLazyListView

We should tackle this as a whole covering all of these cases:


Affected Classes

  • ScrollView
    • unsure if this one as the super class should remain, based on comments above
  • ListView
  • GridView
  • CustomScrollView
  • ReorderableListView
  • AnimatedList
  • ReorderableList
  • BoxScrollView

Primary Tasks

  • Identify remaining shrinkWrap cases in the framework - refactor if possible
  • Create design doc that goes into more detail on the new classes, migration plan, and everything covered here
  • Add new classes (whatever they may be called)
  • Migrate any remaining shrinkWrap in framework, or in use by customers to the new classes
  • Deprecate shrinkWrap in all scrolling widgets - referring to new classes
  • Document:

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important issues not at the top of the work listc: new featureNothing broken; request for a new capabilityc: proposalA detailed proposal for a change to Flutterf: scrollingViewports, list views, slivers, etc.frameworkflutter/packages/flutter repository. See also f: labels.perf: speedPerformance issues related to (mostly rendering) speedteam-frameworkOwned by Framework teamtriaged-frameworkTriaged by Framework team

    Type

    No type

    Projects

    Status

    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions