Merge pull request #357 from Robert-Frampton/pr-346

Do not favor undefined over default values when updating data manager
metal · Feb 9, 2018 · 664eecc · 664eecc
2 parents 7015031 + 37b274c
commit 664eecc
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 2 deletions.
diff --git a/packages/metal-component/src/ComponentDataManager.js b/packages/metal-component/src/ComponentDataManager.js
@@ -1,6 +1,6 @@
 'use strict';
 
-import {object} from 'metal';
+import {isDef, object} from 'metal';
 import State from 'metal-state';
 
 const BLACKLIST = {
@@ -120,7 +120,7 @@ class ComponentDataManager {
 		for (let i = 0; i < keys.length; i++) {
 			const key = keys[i];
 			if (!state.getStateKeyConfig(key).internal) {
-				if (data.hasOwnProperty(key)) {
+				if (data.hasOwnProperty(key) && isDef(data[key])) {
 					state.set(key, data[key]);
 				} else {
 					state.setDefaultValue(key);

diff --git a/packages/metal-component/test/ComponentDataManager.js b/packages/metal-component/test/ComponentDataManager.js
@@ -101,6 +101,27 @@ describe('ComponentDataManager', function() {
 		assert.strictEqual('bar', component.bar);
 	});
 
+	it('should not favor undefined values over default values when replacing all non internal data', function() {
+		component.constructor.STATE = {
+			bar: {
+				value: 'initialBar',
+			},
+			foo: {
+				value: 'initialFoo',
+			},
+		};
+
+		initialConfig = {};
+		ComponentDataManager.setUp(component, {});
+
+		ComponentDataManager.replaceNonInternal(component, {
+			bar: undefined,
+			foo: 'newFoo',
+		});
+		assert.strictEqual('newFoo', component.foo);
+		assert.strictEqual('initialBar', component.bar);
+	});
+
 	it('should return state instance', function() {
 		ComponentDataManager.setUp(component, {});
 		assert.ok(

diff --git a/packages/metal-jsx/test/JSXComponent.js b/packages/metal-jsx/test/JSXComponent.js
@@ -487,6 +487,55 @@ describe('JSXComponent', function() {
 			assert.ok(dom.hasClass(component.element, 'parent'));
 			assert.ok(dom.hasClass(component.element, 'grandchild'));
 		});
+
+		it('should use default prop value when "undefined" is passed as a value on update', function(
+			done
+		) {
+			class ChildComponent extends JSXComponent {
+				render() {
+					return <div />;
+				}
+			}
+			ChildComponent.PROPS = {
+				childFoo: {
+					value: 'foo',
+				},
+				childBar: {
+					value: 'bar',
+				},
+			};
+
+			class ParentComponent extends JSXComponent {
+				render() {
+					return (
+						<ChildComponent
+							childBar={this.state.bar}
+							childFoo={this.state.foo}
+							ref="childComponent"
+						/>
+					);
+				}
+			}
+			ParentComponent.STATE = {
+				bar: {
+					value: 'bar',
+				},
+				foo: {},
+			};
+
+			component = new ParentComponent();
+
+			component.state.bar = 'newBar';
+
+			component.once('stateChanged', function() {
+				assert.strictEqual(
+					component.refs.childComponent.props.childBar,
+					'newBar'
+				);
+				assert.strictEqual(component.refs.childComponent.props.childFoo, 'foo');
+				done();
+			});
+		});
 	});
 
 	describe('shouldUpdate', function() {