Merge pull request #262 from Workiva/backpatch_map_warnings

CPLAT-4749 Release over_react 1.32.0

Author: rmconsole6-wk

CHANGELOG.md

@@ -1,5 +1,13 @@
 # OverReact Changelog
 
+## 1.32.0
+
+> [Complete `1.32.0` Changeset](https://github.com/Workiva/over_react/compare/1.31.0...1.32.0)
+
+* [#249] Warn consumers about props / state mutation 
+  * Directly mutating props and state is an antipattern and can cause unpredictable rendering.
+      Avoiding this will be especially important for components to behave correctly in React 16's [concurrent mode](https://reactjs.org/blog/2018/11/27/react-16-roadmap.html#react-16x-q2-2019-the-one-with-concurrent-mode).
+
 ## 1.31.0
 
 > [Complete `1.31.0` Changeset](https://github.com/Workiva/over_react/compare/1.30.2...1.31.0)

lib/src/component_declaration/component_base.dart

@@ -16,6 +16,7 @@ library over_react.component_declaration.component_base;
 
 import 'dart:async';
 import 'dart:collection';
+import 'dart:html';
 
 import 'package:meta/meta.dart';
 import 'package:over_react/over_react.dart';
@@ -247,7 +248,7 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component imple
     var unwrappedProps = this.unwrappedProps;
     var typedProps = _typedPropsCache[unwrappedProps];
     if (typedProps == null) {
-      typedProps = typedPropsFactory(unwrappedProps);
+      typedProps = typedPropsFactory(inReactDevMode ? new _WarnOnModify(unwrappedProps, true) : unwrappedProps);
       _typedPropsCache[unwrappedProps] = typedProps;
     }
     return typedProps;
@@ -417,7 +418,7 @@ abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiStat
     var unwrappedState = this.unwrappedState;
     var typedState = _typedStateCache[unwrappedState];
     if (typedState == null) {
-      typedState = typedStateFactory(unwrappedState);
+    typedState = typedStateFactory(inReactDevMode ? new _WarnOnModify(unwrappedState, false) : unwrappedState);
       _typedStateCache[unwrappedState] = typedState;
     }
     return typedState;
@@ -446,6 +447,37 @@ abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiStat
   // ----------------------------------------------------------------------
 }
 
+class _WarnOnModify<K, V> extends MapView<K, V> {
+  //Used to customize warning based on whether the data is props or state
+  bool isProps;
+
+  String message;
+
+  _WarnOnModify(Map componentData, this.isProps): super(componentData);
+
+  @override
+  operator []=(K key, V value) {
+    if (isProps) {
+      message =
+        '''
+          props["$key"] was updated incorrectly. Never mutate this.props directly, as it can cause unexpected behavior; 
+          props must be updated only by passing in new values when re-rendering this component.
+
+          This will throw in UiComponentV2 (to be released as part of the React 16 upgrade).
+        ''';
+    } else {
+      message =
+        '''
+          state["$key"] was updated incorrectly. Never mutate this.state directly, as it can cause unexpected behavior; 
+          state must be updated only via setState.
+
+          This will throw in UiComponentV2 (to be released as part of the React 16 upgrade).
+        ''';
+    }
+    super[key] = value;
+    ValidationUtil.warn(unindent(message));
+  }
+}
 
 /// A `dart.collection.MapView`-like class with strongly-typed getters/setters for React state.
 ///

pubspec.yaml

@@ -1,5 +1,5 @@
 name: over_react
-version: 1.31.0
+version: 1.32.0
 description: A library for building statically-typed React UI components using Dart.
 homepage: https://github.com/Workiva/over_react/
 authors:

test/over_react/component_declaration/component_base_test.dart

@@ -124,6 +124,16 @@ main() {
         _commonNonInvokedBuilderTests(Dom.div());
       }, testOn: '!js');
 
+      test('warns against setting props directly', () {
+        startRecordingValidationWarnings();
+        var instance = render(TestComponent()());
+        var component = getDartComponent(instance);
+        var changeProps = () => component.props['id'] = 'test';
+        changeProps();
+        verifyValidationWarning(contains('Never mutate this.props directly'));
+        stopRecordingValidationWarnings();
+      });
+
       group('renders a DOM component with the correct children when', () {
         _commonVariadicChildrenTests(Dom.div());
 
@@ -829,7 +839,7 @@ main() {
 
       setUp(() {
         statefulComponent = new TestStatefulComponentComponent();
-        statefulComponent.unwrappedState = {};
+        statefulComponent.unwrappedState = {'test': true};
       });
 
       group('`state`', () {
@@ -856,6 +866,14 @@ main() {
 
             expect(stateBeforeChange, isNot(same(stateAfterChange)));
           });
+
+          test('warns against setting state directly', () {
+            startRecordingValidationWarnings();
+            var changeState = () => statefulComponent.state['test'] = true;
+            changeState();
+            verifyValidationWarning(contains('Never mutate this.state directly'));
+            stopRecordingValidationWarnings();
+          });
         });
 
         group('setter:', () {