Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[flutter_adaptive_scaffold] unnecessary rebuilds when checking if breakpoint is active #146801

Closed
waleedf112 opened this issue Apr 16, 2024 · 5 comments · Fixed by flutter/packages#6544
Closed

[flutter_adaptive_scaffold] unnecessary rebuilds when checking if breakpoint is active #146801

waleedf112 opened this issue Apr 16, 2024 · 5 comments · Fixed by flutter/packages#6544
Labels
found in release: 3.19 Found to occur in 3.19 found in release: 3.22 Found to occur in 3.22 has reproducible steps The issue has been confirmed reproducible and is ready to work on p: flutter_adaptive_scaffold The flutter_adaptive_scaffold package P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. r: fixed Issue is closed as already fixed in a newer version team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team

Comments

@waleedf112
Copy link

waleedf112 commented Apr 16, 2024 •
edited

Steps to reproduce

  1. invoke Breakpoints.<BREAK-POINT>.isActive(context) somewhere in your code.
  2. interact with the app in a way that would change any property of MediaQuery, like changing the OS theme, opening the keyboard, etc.

Code sample

Code sample 
class MyScreen extends StatefulWidget {
  const MyScreen({super.key});

  @override
  State<MyScreen> createState() => _MyScreenState();
}

class _MyScreenState extends State<MyScreen> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: [
            Text('last rebuild: ${DateTime.now()}'),
            const TextField(
              decoration: InputDecoration(hintText: 'open keyboard to trigger rebuilds'),
            ),
            FilledButton(
              onPressed: () {
                Breakpoints.small.isActive(context);
              },
              child: const Text('press to trigger the bug'),
            ),
          ],
        ),
      ),
    );
  }
}

What target platforms are you seeing this bug on?

Android, iOS

OS/Browser name and version | Device information

Android 14

Does the problem occur on emulator/simulator as well as on physical devices?

Yes

Logs

No response

Flutter Doctor output

Doctor output 
[✓] Flutter (Channel stable, 3.19.5, on macOS 14.3.1 23D60 darwin-arm64, locale en-SA)
    • Flutter version 3.19.5 on channel stable at /Users/waleed/Development/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 300451adae (3 weeks ago), 2024-03-27 21:54:07 -0500
    • Engine revision e76c956498
    • Dart version 3.3.3
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/waleed/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15C500b
    • CocoaPods version 1.15.2

[✗] Chrome - develop for the web (Cannot find Chrome executable at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome)
    ! Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable.

[✓] Android Studio (version 2023.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)

[✓] VS Code (version 1.86.2)
    • VS Code at /Users/waleed/Downloads/Visual Studio Code.app/Contents
    • Flutter extension version 3.86.0

[✓] Connected device (3 available)
    • SM F731B (mobile)           • R5CW7235F3F   • android-arm64 • Android 14 (API 34)
    • sdk gphone64 arm64 (mobile) • emulator-5554 • android-arm64 • Android 14 (API 34) (emulator)
    • macOS (desktop)             • macos         • darwin-arm64  • macOS 14.3.1 23D60 darwin-arm64

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.
@waleedf112 waleedf112 added the from: performance template Issues created via a performance issue template label Apr 16, 2024
@waleedf112 waleedf112 mentioned this issue Apr 16, 2024
Closed
@danagbemava-nc danagbemava-nc removed the from: performance template Issues created via a performance issue template label Apr 16, 2024
@waleedf112 waleedf112 mentioned this issue Apr 16, 2024
Merged
11 tasks
@darshankawar darshankawar added the in triage Presently being triaged by the triage team label Apr 16, 2024
@darshankawar
Copy link
Member

@waleedf112
I tried your code sample and observed that if we open and close keyboard, the timestamp changes. Is this what you are calling as unnecessary rebuild ?
If so, please check if this resembles your case or not, which also discusses about MediaQuery.of(context).

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 16, 2024
@waleedf112
Copy link
Author

@waleedf112 I tried your code sample and observed that if we open and close keyboard, the timestamp changes. Is this what you are calling as unnecessary rebuild ? If so, please check if this resembles your case or not, which also discusses about MediaQuery.of(context).

they are similar, it's not only caused by the keyboard, changing the device's theme, system language, device orientation, also triggers a rebuild.
the root cause is the same, it's MediaQuery.of(context) that gets invoked within the isActive(context) method, modifying it to MediaQuery.sizeOf(context) fixes the issue.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 16, 2024
@darshankawar
Copy link
Member

Thanks for the update. Is it only occuring or specific to flutter_adaptive_scaffold or just using MediaQuery.of(context), and changing the device's theme, system language, device orientation, also triggers a rebuild ?

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 17, 2024
@waleedf112
Copy link
Author

waleedf112 commented Apr 17, 2024 •
edited

Thanks for the update. Is it only occuring or specific to flutter_adaptive_scaffold or just using MediaQuery.of(context), and changing the device's theme, system language, device orientation, also triggers a rebuild ?

@darshankawar

MediaQuery.of(context) triggers a rebuild whenever any property changes like the device theme, language, etc. that's the expected behavior, but within the scope of flutter_adaptive_scaffold's method breakpoint.isActive(context), the only trigger should be when the width of the device screen changes, there's no need to rebuild the whole widget tree and keep listening to MediaQuery and trigger unnecessary rebuilds.

look at this PR.

as a user of the flutter_adaptive_scaffold package, I have no control over the rebuilds caused by the package.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 17, 2024
@darshankawar
Copy link
Member

Thanks for the update and potential fix. I was able to replicate the reported behavior.

stable, master flutter doctor -v 
[!] Flutter (Channel stable, 3.19.5, on macOS 12.2.1 21D62 darwin-x64, locale
    en-GB)
    • Flutter version 3.19.5 on channel stable at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 300451adae (4 days ago), 2024-03-27 21:54:07 -0500
    • Engine revision e76c956498
    • Dart version 3.3.3
    • DevTools version 2.31.1
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

[!] Flutter (Channel master, 3.22.0-11.0.pre.15, on macOS 12.2.1 21D62
    darwin-x64, locale en-GB)
    • Flutter version 3.22.0-11.0.pre.15 on channel master at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 08bae320c5 (86 minutes ago), 2024-04-16 00:06:18 -0400
    • Engine revision 66ad802178
    • Dart version 3.5.0 (build 3.5.0-61.0.dev)
    • DevTools version 2.35.0-dev.8
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[!] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    • Android SDK at /Users/dhs/Library/Android/sdk
    ✗ cmdline-tools component is missing
      Run `path/to/sdkmanager --install "cmdline-tools;latest"`
      See https://developer.android.com/studio/command-line for more details.
    ✗ Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/macos#android-setup for
      more details.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 13C100
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] IntelliJ IDEA Ultimate Edition (version 2021.3.2)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    • Flutter plugin version 65.1.4
    • Dart plugin version 213.7228

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.29.0

[✓] Connected device (3 available)
    • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios
      • iOS 15.3.1 19D52
    • macOS (desktop)           • macos                                    •
      darwin-x64     • macOS 12.2.1 21D62 darwin-x64
    • Chrome (web)              • chrome                                   •
      web-javascript • Google Chrome 109.0.5414.119

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.
      
[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

@darshankawar darshankawar added package flutter/packages repository. See also p: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on p: flutter_adaptive_scaffold The flutter_adaptive_scaffold package found in release: 3.19 Found to occur in 3.19 found in release: 3.22 Found to occur in 3.22 team-ecosystem Owned by Ecosystem team and removed in triage Presently being triaged by the triage team labels Apr 17, 2024
@stuartmorgan stuartmorgan added P2 Important issues not at the top of the work list triaged-ecosystem Triaged by Ecosystem team labels Apr 23, 2024
@darshankawar darshankawar added the r: fixed Issue is closed as already fixed in a newer version label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
found in release: 3.19 Found to occur in 3.19 found in release: 3.22 Found to occur in 3.22 has reproducible steps The issue has been confirmed reproducible and is ready to work on p: flutter_adaptive_scaffold The flutter_adaptive_scaffold package P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. r: fixed Issue is closed as already fixed in a newer version team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team
Projects
None yet
Milestone
No milestone
4 participants
@stuartmorgan @waleedf112 @darshankawar @danagbemava-nc