From 6b481d3d32ebc1d4fdb4a97895e78ee705a69b11 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Fri, 5 Apr 2024 14:44:19 +0200 Subject: [PATCH] Add warning when LocalizationsDelegate.load() is async (#342) --- lib/src/core/wiredash_widget.dart | 35 +++++++ test/issues/issue_341_test.dart | 160 ++++++++++++++++++++++++++++++ test/util/flutter_error.dart | 31 ++++-- 3 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 test/issues/issue_341_test.dart diff --git a/lib/src/core/wiredash_widget.dart b/lib/src/core/wiredash_widget.dart index c1dd8618..bc938e67 100644 --- a/lib/src/core/wiredash_widget.dart +++ b/lib/src/core/wiredash_widget.dart @@ -190,6 +190,9 @@ class WiredashState extends State { projectId: widget.projectId, secret: widget.secret, ); + + _verifySyncLocalizationsDelegate(); + _services.updateWidget(widget); _services.addListener(_markNeedsBuild); _services.wiredashModel.addListener(_markNeedsBuild); @@ -229,6 +232,10 @@ class WiredashState extends State { secret: widget.secret, ); } + if (oldWidget.options?.localizationDelegate != + widget.options?.localizationDelegate) { + _verifySyncLocalizationsDelegate(); + } _services.updateWidget(widget); } @@ -383,6 +390,34 @@ class WiredashState extends State { // Use what's set by the operating system return _defaultLocale; } + + void _verifySyncLocalizationsDelegate() { + assert(() { + final delegate = widget.options?.localizationDelegate; + if (delegate == null) { + return true; + } + final locale = _currentLocale; + if (!delegate.isSupported(locale)) { + // load should not be called + return true; + } + final loadFuture = delegate.load(locale); + if (loadFuture is! SynchronousFuture) { + reportWiredashInfo( + 'Warning: $delegate load() is async', + StackTrace.current, + 'Warning: ${delegate.runtimeType}.load() returned a Future for Locale "$locale".\n' + 'This will lead to your app losing all its state when you open Wiredash!\n' + '\tDO return a SynchronousFuture from your LocalizationsDelegate.load() method. \n' + '\tDO NOT use the async keyword in LocalizationsDelegate.load().\n' + 'When load() returns SynchronousFuture the Localizations widget can build your app widget with the already loaded localizations at the first frame.\n' + 'For more information visit https://github.com/wiredashio/wiredash-sdk/issues/341', + ); + } + return true; + }()); + } } Locale get _defaultLocale { diff --git a/test/issues/issue_341_test.dart b/test/issues/issue_341_test.dart new file mode 100644 index 00000000..cce038b7 --- /dev/null +++ b/test/issues/issue_341_test.dart @@ -0,0 +1,160 @@ +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:wiredash/wiredash.dart'; + +import '../util/flutter_error.dart'; + +void main() { + group('issue 341', () { + testWidgets('no LocalizationsDelegate', (tester) async { + await tester.pumpWidget(const MyApp()); + await tester.pumpAndSettle(); + expect(TestWidget.createCount, 1); + await tester.tap(find.byType(FloatingActionButton)); + await tester.pumpAndSettle(); + expect(TestWidget.createCount, 1); // Nice! + }); + + testWidgets('LocalizationsDelegate with SynchronousFuture', (tester) async { + await tester.pumpWidget(const MyApp(asyncDelegate: false)); + await tester.pumpAndSettle(); + expect(TestWidget.createCount, 1); + await tester.tap(find.byType(FloatingActionButton)); + await tester.pumpAndSettle(); + expect(TestWidget.createCount, 1); // Nice! + }); + + testWidgets('async LocalizationsDelegate', (tester) async { + final errors = captureFlutterErrors(); + await tester.pumpWidget(const MyApp(asyncDelegate: true)); + await tester.pumpAndSettle(); + expect(TestWidget.createCount, 1); + await tester.tap(find.byType(FloatingActionButton)); + await tester.pumpAndSettle(); + errors.restoreDefaultErrorHandlers(); + // This causes the TestWidget to lose its state + expect(TestWidget.createCount, 2); + expect(errors.presentErrorText, contains("SynchronousFuture")); + expect( + errors.presentErrorText, + contains("AsyncCustomWiredashTranslationsDelegate"), + ); + expect(errors.presentError.length, 1); + }); + }); +} + +class MyApp extends StatefulWidget { + const MyApp({ + super.key, + this.asyncDelegate, + }); + + @override + State createState() => _MyAppState(); + + final bool? asyncDelegate; +} + +class _MyAppState extends State { + @override + Widget build(BuildContext context) { + return Wiredash( + projectId: "xxxxx", + secret: "xxxxx", + options: WiredashOptionsData( + localizationDelegate: widget.asyncDelegate == null + ? null + : widget.asyncDelegate == true + ? const AsyncCustomWiredashTranslationsDelegate() + : const SyncCustomWiredashTranslationsDelegate(), + ), + child: const MaterialApp( + home: TestWidget(), + ), + ); + } +} + +class TestWidget extends StatefulWidget { + const TestWidget({super.key}); + + @override + State createState() => _TestWidgetState(); + + static int createCount = 0; +} + +class _TestWidgetState extends State { + @override + void initState() { + super.initState(); + debugPrint("TestWidget initState"); + addTearDown(() { + TestWidget.createCount = 0; + }); + TestWidget.createCount++; + } + + @override + Widget build(BuildContext context) { + return Scaffold( + body: const Placeholder(), + floatingActionButton: FloatingActionButton( + onPressed: () => Wiredash.of(context).show(), + ), + ); + } +} + +class AsyncCustomWiredashTranslationsDelegate + extends LocalizationsDelegate { + const AsyncCustomWiredashTranslationsDelegate(); + + @override + bool isSupported(Locale locale) { + return ['en'].contains(locale.languageCode); + } + + @override + Future load(Locale locale) async { + switch (locale.languageCode) { + case 'en': + return _EnOverrides(); + default: + throw "Unsupported locale $locale"; + } + } + + @override + bool shouldReload(AsyncCustomWiredashTranslationsDelegate old) => false; +} + +class SyncCustomWiredashTranslationsDelegate + extends LocalizationsDelegate { + const SyncCustomWiredashTranslationsDelegate(); + + @override + bool isSupported(Locale locale) { + return ['en'].contains(locale.languageCode); + } + + @override + Future load(Locale locale) { + switch (locale.languageCode) { + case 'en': + return SynchronousFuture(_EnOverrides()); + default: + throw "Unsupported locale $locale"; + } + } + + @override + bool shouldReload(SyncCustomWiredashTranslationsDelegate old) => false; +} + +class _EnOverrides extends WiredashLocalizationsEn { + @override + String get feedbackStep1MessageHint => 'Test'; +} diff --git a/test/util/flutter_error.dart b/test/util/flutter_error.dart index 440f6cb7..ebd7f1ef 100644 --- a/test/util/flutter_error.dart +++ b/test/util/flutter_error.dart @@ -4,21 +4,16 @@ import 'package:flutter_test/flutter_test.dart'; /// Consumes all [FlutterError.onError] and [FlutterError.presentError] calls /// during this test and makes them accessible as list for assertions. FlutterErrors captureFlutterErrors() { - final errors = FlutterErrors(); - final oldPresentHandler = FlutterError.presentError; + final errors = FlutterErrors(FlutterError.onError, FlutterError.presentError); FlutterError.presentError = (details) { errors._presentError.add(details); }; - addTearDown(() { - FlutterError.presentError = oldPresentHandler; - }); - - final oldOnErrorHandler = FlutterError.onError; FlutterError.onError = (FlutterErrorDetails details) { errors._onError.add(details); }; + addTearDown(() { - FlutterError.onError = oldOnErrorHandler; + errors.restoreDefaultErrorHandlers(); }); return errors; @@ -26,10 +21,30 @@ FlutterErrors captureFlutterErrors() { /// A summary of [FlutterError.onError] and [FlutterError.presentError] calls class FlutterErrors { + final FlutterExceptionHandler? _originalOnError; + final FlutterExceptionHandler _originalPresentError; + + FlutterErrors( + this._originalOnError, + this._originalPresentError, + ); + List get onError => List.unmodifiable(_onError); final List _onError = []; List get presentError => List.unmodifiable(_presentError); final List _presentError = []; + + void restoreDefaultErrorHandlers() { + FlutterError.onError = _originalOnError; + FlutterError.presentError = _originalPresentError; + } + + String get presentErrorText { + final renderer = TextTreeRenderer(wrapWidth: 100000); + return presentError.map((e) { + return renderer.render(e.toDiagnosticsNode()); + }).join('\n'); + } }