From 58811028a4896222ff85acec842d664f878fefe9 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 30 Jan 2020 07:58:03 -0800 Subject: [PATCH] [flutter] Allow hot reload replacements of Stateless/Stateful Widget (#48932) --- .../flutter/lib/src/widgets/framework.dart | 98 +++++++++++++++---- .../stateless_stateful_hot_reload_test.dart | 53 ++++++++++ .../test_data/stateless_stateful_project.dart | 79 +++++++++++++++ .../test/integration.shard/test_utils.dart | 3 +- 4 files changed, 214 insertions(+), 19 deletions(-) create mode 100644 packages/flutter_tools/test/integration.shard/stateless_stateful_hot_reload_test.dart create mode 100644 packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_project.dart diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index 8c432c67ae..d2a53d36a3 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -11,6 +11,7 @@ import 'package:flutter/rendering.dart'; import 'debug.dart'; import 'focus_manager.dart'; +import 'inherited_model.dart'; export 'dart:ui' show hashValues, hashList; @@ -473,6 +474,24 @@ abstract class Widget extends DiagnosticableTree { return oldWidget.runtimeType == newWidget.runtimeType && oldWidget.key == newWidget.key; } + + // Return a numeric encoding of the specific `Widget` concrete subtype. + // This is used in `Element.updateChild` to determine if a hot reload modified the + // superclass of a mounted element's configuration. The encoding of each `Widget` + // must match the corresponding `Element` encoding in `Element._debugConcreteSubtype`. + static int _debugConcreteSubtype(Widget widget) { + return widget is StatefulWidget ? 1 : + widget is StatelessWidget ? 2 : + widget is InheritedModel ? 3 : + widget is InheritedWidget ? 4 : + widget is ParentDataWidget ? 5 : + widget is ProxyWidget ? 6 : + widget is LeafRenderObjectWidget ? 7 : + widget is SingleChildRenderObjectWidget? 8 : + widget is MultiChildRenderObjectWidget ? 9 : + widget is RenderObjectWidget ? 10 : + 0; + } } /// A widget that does not require mutable state. @@ -2838,6 +2857,24 @@ abstract class Element extends DiagnosticableTree implements BuildContext { return 0; } + // Return a numeric encoding of the specific `Element` concrete subtype. + // This is used in `Element.updateChild` to determine if a hot reload modified the + // superclass of a mounted element's configuration. The encoding of each `Element` + // must match the corresponding `Widget` encoding in `Widget._debugConcreteSubtype`. + static int _debugConcreteSubtype(Element element) { + return element is StatefulElement ? 1 : + element is StatelessElement ? 2 : + element is InheritedModelElement ? 3 : + element is InheritedElement ? 4 : + element is ParentDataElement ? 5 : + element is ProxyElement ? 6 : + element is LeafRenderObjectElement ? 7 : + element is SingleChildRenderObjectElement ? 8 : + element is MultiChildRenderObjectElement ? 9 : + element is RenderObjectElement ? 10 : + 0; + } + /// The configuration for this element. @override Widget get widget => _widget; @@ -3069,21 +3106,45 @@ abstract class Element extends DiagnosticableTree implements BuildContext { return null; } if (child != null) { - if (child.widget == newWidget) { - if (child.slot != newSlot) - updateSlotForChild(child, newSlot); - return child; - } - if (Widget.canUpdate(child.widget, newWidget)) { - if (child.slot != newSlot) - updateSlotForChild(child, newSlot); - child.update(newWidget); - assert(child.widget == newWidget); - assert(() { - child.owner._debugElementWasRebuilt(child); - return true; - }()); - return child; + bool hasSameSuperclass = true; + // When the type of a widget is changed between Stateful and Stateless via + // hot reload, the element tree will end up in a partially invalid state. + // That is, if the widget was a StatefulWidget and is now a StatelessWidget, + // then the element tree currently contains a StatefulElement that is incorrectly + // referencing a StatelessWidget (and likewise with StatelessElement). + // + // To avoid crashing due to type errors, we need to gently guide the invalid + // element out of the tree. To do so, we ensure that the `hasSameSuperclass` condition + // returns false which prevents us from trying to update the existing element + // incorrectly. + // + // For the case where the widget becomes Stateful, we also need to avoid + // accessing `StatelessElement.widget` as the cast on the getter will + // cause a type error to be thrown. Here we avoid that by short-circuiting + // the `Widget.canUpdate` check once `hasSameSuperclass` is false. + assert(() { + final int oldElementClass = Element._debugConcreteSubtype(child); + final int newWidgetClass = Widget._debugConcreteSubtype(newWidget); + hasSameSuperclass = oldElementClass == newWidgetClass; + return true; + }()); + if (hasSameSuperclass) { + if (child.widget == newWidget) { + if (child.slot != newSlot) + updateSlotForChild(child, newSlot); + return child; + } + if (Widget.canUpdate(child.widget, newWidget)) { + if (child.slot != newSlot) + updateSlotForChild(child, newSlot); + child.update(newWidget); + assert(child.widget == newWidget); + assert(() { + child.owner._debugElementWasRebuilt(child); + return true; + }()); + return child; + } } deactivateChild(child); assert(child._parent == null); @@ -3424,7 +3485,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { @mustCallSuper void deactivate() { assert(_debugLifecycleState == _ElementLifecycle.active); - assert(widget != null); + assert(_widget != null); // Use the private property to avoid a CastError during hot reload. assert(depth != null); assert(_active); if (_dependencies != null && _dependencies.isNotEmpty) { @@ -3467,10 +3528,11 @@ abstract class Element extends DiagnosticableTree implements BuildContext { @mustCallSuper void unmount() { assert(_debugLifecycleState == _ElementLifecycle.inactive); - assert(widget != null); + assert(_widget != null); // Use the private property to avoid a CastError during hot reload. assert(depth != null); assert(!_active); - final Key key = widget.key; + // Use the private property to avoid a CastError during hot reload. + final Key key = _widget.key; if (key is GlobalKey) { key._unregister(this); } diff --git a/packages/flutter_tools/test/integration.shard/stateless_stateful_hot_reload_test.dart b/packages/flutter_tools/test/integration.shard/stateless_stateful_hot_reload_test.dart new file mode 100644 index 0000000000..01f8728338 --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/stateless_stateful_hot_reload_test.dart @@ -0,0 +1,53 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'package:file/file.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; + +import '../src/common.dart'; +import 'test_data/stateless_stateful_project.dart'; +import 'test_driver.dart'; +import 'test_utils.dart'; + +// This test verifies that we can hot reload a stateless widget into a +// stateful one and back. +void main() { + Directory tempDir; + final HotReloadProject _project = HotReloadProject(); + FlutterRunTestDriver _flutter; + + setUp(() async { + tempDir = createResolvedTempDirectorySync('hot_reload_test.'); + await _project.setUpIn(tempDir); + _flutter = FlutterRunTestDriver(tempDir); + }); + + tearDown(() async { + await _flutter?.stop(); + tryToDelete(tempDir); + }); + + test('Can switch between stateless and stateful', () async { + await _flutter.run(); + await _flutter.hotReload(); + final StringBuffer stdout = StringBuffer(); + final StreamSubscription subscription = _flutter.stdout.listen(stdout.writeln); + + // switch to stateful. + _project.toggleState(); + await _flutter.hotReload(); + + // switch to stateless. + _project.toggleState(); + await _flutter.hotReload(); + + final String logs = stdout.toString(); + + expect(logs, contains('STATELESS')); + expect(logs, contains('STATEFUL')); + await subscription.cancel(); + }); +} diff --git a/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_project.dart b/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_project.dart new file mode 100644 index 0000000000..601ef8c328 --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_project.dart @@ -0,0 +1,79 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter_tools/src/globals.dart' as globals; + +import '../test_utils.dart'; +import 'project.dart'; + +class HotReloadProject extends Project { + @override + final String pubspec = ''' + name: test + environment: + sdk: ">=2.0.0-dev.68.0 <3.0.0" + + dependencies: + flutter: + sdk: flutter + '''; + + @override + final String main = getCode(false); + + static String getCode(bool stateful) { + return ''' + import 'package:flutter/material.dart'; + import 'package:flutter/scheduler.dart'; + import 'package:flutter/services.dart'; + import 'package:flutter/widgets.dart'; + + void main() async { + WidgetsFlutterBinding.ensureInitialized(); + final ByteData message = const StringCodec().encodeMessage('AppLifecycleState.resumed'); + await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage('flutter/lifecycle', message, (_) { }); + runApp(MyApp()); + } + + class MyApp extends StatelessWidget { + @override + Widget build(BuildContext context) { + return MaterialApp( + title: 'Flutter Demo', + home: Child(), + ); + } + } + + + class ${stateful ? 'Other' : 'Child'} extends StatelessWidget { + @override + Widget build(BuildContext context) { + print('STATELESS'); + return Container(); + } + } + + class ${stateful ? 'Child' : 'Other'} extends StatefulWidget { + State createState() => _State(); + } + + class _State extends State<${stateful ? 'Child' : 'Other'}>{ + @override + Widget build(BuildContext context) { + print('STATEFUL'); + return Container(); + } + } + '''; + } + + /// Whether the template is currently stateful. + bool stateful = false; + + void toggleState() { + stateful = !stateful; + writeFile(globals.fs.path.join(dir.path, 'lib', 'main.dart'), getCode(stateful)); + } +} diff --git a/packages/flutter_tools/test/integration.shard/test_utils.dart b/packages/flutter_tools/test/integration.shard/test_utils.dart index 5aa754965a..d2b2e9fadd 100644 --- a/packages/flutter_tools/test/integration.shard/test_utils.dart +++ b/packages/flutter_tools/test/integration.shard/test_utils.dart @@ -22,7 +22,8 @@ Directory createResolvedTempDirectorySync(String prefix) { void writeFile(String path, String content) { globals.fs.file(path) ..createSync(recursive: true) - ..writeAsStringSync(content); + ..writeAsStringSync(content) + ..setLastModifiedSync(DateTime.now().add(const Duration(seconds: 10))); } void writePackages(String folder) {