Skip to content

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

Open

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:

Activity

  1. added
    c: new featureNothing broken; request for a new capability
    frameworkflutter/packages/flutter repository. See also f: labels.
    f: scrollingViewports, list views, slivers, etc.
    c: proposalA detailed proposal for a change to Flutter
    perf: speedPerformance issues related to (mostly rendering) speed
    P1High-priority issues at the top of the work list
    on Jul 11, 2023
  2. Piinks commented on Jul 12, 2023

    @Piinks
    ContributorAuthor

    FYI: I added a fifth scenario above for developers to refactor away from shrinkwrap.

  3. removed their assignment
    on Aug 8, 2023
  4. omartinma commented on Aug 30, 2023

    @omartinma

    @Piinks Do we have an estimation of when you think this change will be effective? Thanks!

  5. erlangparasu commented on Oct 14, 2023

    @erlangparasu

    Any other widget properties that performance trade-off other than shrinkWrap ?

  6. moved this from In Progress to Todo in Scrolling Performance Pitfallson Nov 3, 2023
  7. YoussefLasheen commented on Dec 26, 2023

    @YoussefLasheen

    I was just having a problem with scrolling performance today and it took me like 2 hours to figure out that the shrinkwrap flag was the cause by trial and error. So good work.

  8. flutter-triage-bot commented on Jan 18, 2024

    @flutter-triage-bot

    This issue is marked P1 but has had no recent status updates.

    The P1 label indicates high-priority issues that are at the top of the work list. This is the highest priority level a bug can have if it isn't affecting a top-tier customer or breaking the build. Bugs marked P1 are generally actively being worked on unless the assignee is dealing with a P0 bug (or another P1 bug). Issues at this level should be resolved in a matter of months and should have monthly updates on GitHub.

    Please consider where this bug really falls in our current priorities, and label it or assign it accordingly. This allows people to have a clearer picture of what work is actually planned. Thanks!

  9. added
    P2Important issues not at the top of the work list
    and removed
    P1High-priority issues at the top of the work list
    on Jan 18, 2024
  10. anggaaryas commented on Jun 19, 2024

    @anggaaryas

    Our team use custom scroll view and sliver widget to archive similar behavior with shrink wrap. But it too verbose to use it. Curious how other framework handle this behavior (ex, compose, react, etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions