From af7e04390a14111d3dcb48ac9f51931264e19163 Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Tue, 8 Aug 2017 13:31:22 -0700 Subject: [PATCH 1/2] Redraw only once when store triggers along with ancestor rerender --- .../component_declaration/flux_component.dart | 5 ++ pubspec.yaml | 8 ++++ .../flux_component_test.dart | 46 +++++++++++++++++++ .../flux_component_test/nested.dart | 27 +++++++++++ 4 files changed, 86 insertions(+) create mode 100644 test/over_react/component_declaration/flux_component_test/nested.dart diff --git a/lib/src/component_declaration/flux_component.dart b/lib/src/component_declaration/flux_component.dart index 45c7d62f9..b5638ae87 100644 --- a/lib/src/component_declaration/flux_component.dart +++ b/lib/src/component_declaration/flux_component.dart @@ -175,6 +175,11 @@ abstract class _FluxComponentMixin implements Batche }); } + void componentDidUpdate(Map prevProps, Map prevState) { + // Let BatchedRedraws know that this component has redrawn in the current batch + didBatchRedraw = true; + } + void componentWillUnmount() { // Ensure that unmounted components don't batch render shouldBatchRedraw = false; diff --git a/pubspec.yaml b/pubspec.yaml index 9c28c1be5..b3632732e 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -37,3 +37,11 @@ transformers: $include: test/**_test{.*,}.dart # Reminder: dart2js should come after any other transformers that touch Dart code - $dart2js + + +dependency_overrides: + w_flux: + git: + url: git@github.com:greglittlefield-wf/w_flux.git + ref: 189a1f97d95e0e3c446a5192c54e890fa61ebfb4 # from multiple_flux_redraws + diff --git a/test/over_react/component_declaration/flux_component_test.dart b/test/over_react/component_declaration/flux_component_test.dart index ee6e3b0bb..257271e6d 100644 --- a/test/over_react/component_declaration/flux_component_test.dart +++ b/test/over_react/component_declaration/flux_component_test.dart @@ -13,6 +13,7 @@ import '../../test_util/test_util.dart'; part 'flux_component_test/default.dart'; part 'flux_component_test/handler_precedence.dart'; +part 'flux_component_test/nested.dart'; part 'flux_component_test/redraw_on.dart'; part 'flux_component_test/store_handlers.dart'; @@ -164,6 +165,51 @@ void main() { await nextTick(); expect(component.numberOfRedraws, equals(0)); }); + + test('only redraws once in response to a store trigger combined with an ancestor rerendering', () async { + var store = new Store(); + + TestNestedComponent nested0; + TestNestedComponent nested1; + TestNestedComponent nested2; + + render( + (TestNested() + ..store = store + ..ref = (ref) { nested0 = ref; } + )( + (TestNested() + ..store = store + ..ref = (ref) { nested1 = ref; } + )( + (TestNested() + ..store = store + ..ref = (ref) { nested2 = ref; } + )() + ) + ) + ); + expect(nested0.renderCount, 1, reason: 'setup check: initial render'); + expect(nested1.renderCount, 1, reason: 'setup check: initial render'); + expect(nested2.renderCount, 1, reason: 'setup check: initial render'); + + nested0.redraw(); + await nextTick(); + + expect(nested0.renderCount, 2, reason: 'setup check: components should rerender their children'); + expect(nested1.renderCount, 2, reason: 'setup check: components should rerender their children'); + expect(nested2.renderCount, 2, reason: 'setup check: components should rerender their children'); + + store.trigger(); + // Two async gaps just to be safe, since we're + // asserting that additional redraws don't happen. + await nextTick(); + await nextTick(); + + expect(nested0.renderCount, 3, reason: 'should have rerendered once in response to the store triggering'); + expect(nested1.renderCount, 3, reason: 'should have rerendered once in response to the store triggering'); + expect(nested2.renderCount, 3, reason: 'should have rerendered once in response to the store triggering'); + }); }); } diff --git a/test/over_react/component_declaration/flux_component_test/nested.dart b/test/over_react/component_declaration/flux_component_test/nested.dart new file mode 100644 index 000000000..4e9ba468f --- /dev/null +++ b/test/over_react/component_declaration/flux_component_test/nested.dart @@ -0,0 +1,27 @@ +part of over_react.component_declaration.flux_component_test; + +@Factory() +UiFactory TestNested; + +@Props() +class TestNestedProps extends FluxUiProps {} + +@Component() +class TestNestedComponent extends FluxUiComponent { + int renderCount = 0; + + @override + render() { + renderCount++; + + var keyCounter = 0; + var newChildren = props.children.map((child) { + // The keys are consistent across rerenders, so they aren't remounted, + // but cloning the element is necessary for react to consider it changed, + // since it returns new ReactElement instances. + return cloneElement(child, domProps()..key = keyCounter++); + }).toList(); + + return Dom.div()(newChildren); + } +} From 179c237e0f88dbf1de1cabe72b0295fbe1b4bae5 Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Fri, 11 Aug 2017 11:56:21 -0700 Subject: [PATCH 2/2] Bump min w_flux version to pull in BatchedRedraw changes --- pubspec.yaml | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index b3632732e..e37afbdfc 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -17,7 +17,7 @@ dependencies: source_span: "^1.4.0" transformer_utils: "^0.1.1" w_common: "^1.6.0" - w_flux: "^2.7.1" + w_flux: "^2.9.0" platform_detect: "^1.3.2" quiver: ">=0.21.4 <0.26.0" dev_dependencies: @@ -37,11 +37,3 @@ transformers: $include: test/**_test{.*,}.dart # Reminder: dart2js should come after any other transformers that touch Dart code - $dart2js - - -dependency_overrides: - w_flux: - git: - url: git@github.com:greglittlefield-wf/w_flux.git - ref: 189a1f97d95e0e3c446a5192c54e890fa61ebfb4 # from multiple_flux_redraws -