LayoutBuilder's descendant widgets should not rebuild more than once in the same frame #146379
Assignees
Labels
a: tests
"flutter test", flutter_test, or one of our tests
found in release: 3.19
Found to occur in 3.19
found in release: 3.22
Found to occur in 3.22
framework
flutter/packages/flutter repository. See also f: labels.
has reproducible steps
The issue has been confirmed reproducible and is ready to work on
P2
Important issues not at the top of the work list
r: fixed
Issue is closed as already fixed in a newer version
team-framework
Owned by Framework team
triaged-framework
Triaged by Framework team
Comments
|
I was able to replicate this using the test code provided. stable, master flutter doctor -v |
darshankawar
added
a: tests
goderbauer
added
P2
bleroux
changed the title
|
Another case of this issue: testWidgets('duplicate GlobalKey false positive', (WidgetTester tester) async {
final widgetKey = GlobalKey(debugLabel: 'widget key');
final widgetWithKey = Builder(builder: (context) {
Directionality.of(context);
return SizedBox(key: widgetKey);
});
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Row(
children: [
LayoutBuilder(
builder: (context, constraints) => widgetWithKey,
),
],
),
),
);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.rtl,
child: Row(
children: [
LayoutBuilder(
builder: (context, constraints) => const Placeholder(),
),
widgetWithKey,
],
),
),
);
});The framework would incorrectly report that there were duplicate global keys:
because the |
|
I tried to handle cases where layoutbuilders are involved https://docs.google.com/document/d/15U1XDLrP-SXfgeu5DBBsA7MQuFpDUW005Y2ObwmYWIc/edit#heading=h.pub7jnop54q0 looks like i missed some use cases? |
I think in this case it's actually worse than an incorrect assert. The final widget tree is incorrect with the while the correct tree looks like this: The LayoutBuilder stole the keyed SizedBox during the first build, and the SizedBox was then unmounted. |
9 tasks
auto-submit bot
pushed a commit
that referenced
this issue
May 29, 2024
Fixes #146379: introduces `Element.buildScope` which `BuildOwner.buildScope` uses to identify subtrees that need skipping (those with different `BuildScope`s). If `Element.update` calls `updateChild` then dirty children will still be rebuilt regardless of their build scopes. This also introduces `LayoutBuilder.applyDoubleRebuildFix` migration flag which should only live for a week or less. Caveats: `LayoutBuilder`'s render object calls `markNeedsLayout` if a descendant Element is dirty. Since `markNeedsLayout` also implies `markNeedsPaint`, the render object is going to be very repaint/relayout-happy. Tests: Presubmits with the migration flag set to true: https://github.com/flutter/flutter/pull/147856/checks?check_run_id=24629865893
auto-submit bot
added a commit
that referenced
this issue
May 29, 2024
…" (#149279) Reverts: #147856 Initiated by: loic-sharma Reason for reverting: tree is closed with errors like: ``` test/integration.shard/break_on_framework_exceptions_test.dart: breaks when rebuilding dirty elements throws [E] Expected: <45> Actual: <2756> package:matcher expect test\integration.shard\break_on_framework_exceptions_test.dart 56:5 main.expectException ===== asynchronous gap === Original PR Author: LongCatIsLooong Reviewed By: {goderbauer} This change reverts the following previous change: Fixes #146379: introduces `Element.buildScope` which `BuildOwner.buildScope` uses to identify subtrees that need skipping (those with different `BuildScope`s). If `Element.update` calls `updateChild` then dirty children will still be rebuilt regardless of their build scopes. This also introduces `LayoutBuilder.applyDoubleRebuildFix` migration flag which should only live for a week or less. Caveats: `LayoutBuilder`'s render object calls `markNeedsLayout` if a descendant Element is dirty. Since `markNeedsLayout` also implies `markNeedsPaint`, the render object is going to be very repaint/relayout-happy. Tests: Presubmits with the migration flag set to true: https://github.com/flutter/flutter/pull/147856/checks?check_run_id=24629865893
|
@LongCatIsLooong, I'm reopening this as the fix was reverted: #149279 |
victorsanni
pushed a commit
to victorsanni/flutter
that referenced
this issue
May 31, 2024
Fixes flutter#146379: introduces `Element.buildScope` which `BuildOwner.buildScope` uses to identify subtrees that need skipping (those with different `BuildScope`s). If `Element.update` calls `updateChild` then dirty children will still be rebuilt regardless of their build scopes. This also introduces `LayoutBuilder.applyDoubleRebuildFix` migration flag which should only live for a week or less. Caveats: `LayoutBuilder`'s render object calls `markNeedsLayout` if a descendant Element is dirty. Since `markNeedsLayout` also implies `markNeedsPaint`, the render object is going to be very repaint/relayout-happy. Tests: Presubmits with the migration flag set to true: https://github.com/flutter/flutter/pull/147856/checks?check_run_id=24629865893
victorsanni
pushed a commit
to victorsanni/flutter
that referenced
this issue
May 31, 2024
…r#147856)" (flutter#149279) Reverts: flutter#147856 Initiated by: loic-sharma Reason for reverting: tree is closed with errors like: ``` test/integration.shard/break_on_framework_exceptions_test.dart: breaks when rebuilding dirty elements throws [E] Expected: <45> Actual: <2756> package:matcher expect test\integration.shard\break_on_framework_exceptions_test.dart 56:5 main.expectException ===== asynchronous gap === Original PR Author: LongCatIsLooong Reviewed By: {goderbauer} This change reverts the following previous change: Fixes flutter#146379: introduces `Element.buildScope` which `BuildOwner.buildScope` uses to identify subtrees that need skipping (those with different `BuildScope`s). If `Element.update` calls `updateChild` then dirty children will still be rebuilt regardless of their build scopes. This also introduces `LayoutBuilder.applyDoubleRebuildFix` migration flag which should only live for a week or less. Caveats: `LayoutBuilder`'s render object calls `markNeedsLayout` if a descendant Element is dirty. Since `markNeedsLayout` also implies `markNeedsPaint`, the render object is going to be very repaint/relayout-happy. Tests: Presubmits with the migration flag set to true: https://github.com/flutter/flutter/pull/147856/checks?check_run_id=24629865893
darshankawar
added
the
r: fixed
|
FTR, the reland appears to be #149303. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
This came up again in #143249: we can't use a
LayoutBuilderto get the incoming constraints because a google internal test failure caused by this.This becomes problematic when a disposable object (such as a controller) is passed down the widget tree as a widget parameter, across the
LayoutBuilderboundary:ControllerOwner -> LayoutBuilder -> IntermediateWidget -> ControllerUser.
If ControllerUser rebulids without IntermediateWidget rebuilding (as seen in the test), it may keep using an outdated / disposed controller.
The text was updated successfully, but these errors were encountered: