hasInitialValue_ should return false if value is undefined, but valid…

…ators should run regardless. Fixes metal#259
robframpton · Oct 12, 2017 · f373e21 · f373e21
1 parent 449267e
commit f373e21
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 20 deletions.
diff --git a/packages/metal-component/test/ComponentDataManager.js b/packages/metal-component/test/ComponentDataManager.js
@@ -42,6 +42,19 @@ describe('ComponentDataManager', function() {
 		assert.strictEqual('initialFoo', component.foo);
 	});
 
+	it('should use default state value when "undefined" is passed as initial value', function() {
+		initialConfig = {
+			foo: undefined
+		};
+		ComponentDataManager.setUp(component, {
+			foo: {
+				value: 'defaultFoo'
+			}
+		});
+
+		assert.strictEqual(component.foo, 'defaultFoo');
+	});
+
 	it('should throw error if attempting to add state property named "element"', function() {
 		assert.throws(() => {
 			ComponentDataManager.setUp(component, {

diff --git a/packages/metal-jsx/test/JSXDataManager.js b/packages/metal-jsx/test/JSXDataManager.js
@@ -168,6 +168,22 @@ describe('JSXDataManager', function() {
 			assert.strictEqual('foo', component.props.foo);
 		});
 
+		it('should use default prop value when "undefined" is passed as initial value', function() {
+			class TestComponent extends Component {
+			}
+			TestComponent.DATA_MANAGER = JSXDataManager;
+			TestComponent.PROPS = {
+				foo: {
+					value: 'defaultFoo'
+				}
+			};
+
+			component = new TestComponent({
+				foo: undefined
+			});
+			assert.strictEqual('defaultFoo', component.props.foo);
+		});
+
 		it('should include default component data in "props"', function() {
 			class TestComponent extends Component {
 			}
@@ -374,22 +390,5 @@ describe('JSXDataManager', function() {
 			assert.strictEqual('defaultFoo', component.propsChanged.args[0][0].foo);
 			assert.strictEqual('foo', component.props.foo);
 		});
-
-		it('should use default prop value when `undefined` is passed', function() {
-			class TestComponent extends Component {
-			}
-
-			TestComponent.DATA_MANAGER = JSXDataManager;
-
-			TestComponent.PROPS = {
-				foo: {
-					value: 'defaultFoo'
-				}
-			};
-
-			component = new TestComponent({foo: undefined});
-
-			assert.strictEqual('defaultFoo', component.props.foo);
-		});
 	});
 });
diff --git a/packages/metal-state/src/State.js b/packages/metal-state/src/State.js
@@ -1,6 +1,6 @@
 'use strict';
 
-import { async, getStaticProperty, isDefAndNotNull, isFunction, isObject, isString, object } from 'metal';
+import { async, getStaticProperty, isDef, isDefAndNotNull, isFunction, isObject, isString, object } from 'metal';
 import { EventEmitter } from 'metal-events';
 
 /**
@@ -412,7 +412,8 @@ class State extends EventEmitter {
 	 * @protected
 	 */
 	hasInitialValue_(name) {
-		return this.initialValues_.hasOwnProperty(name);
+		return this.initialValues_.hasOwnProperty(name) &&
+			isDef(this.initialValues_[name]);
 	}
 
 	/**
@@ -637,7 +638,9 @@ class State extends EventEmitter {
 	 * @protected
 	 */
 	validateInitialValue_(name) {
-		if (this.hasInitialValue_(name) && !this.callValidator_(name, this.initialValues_[name])) {
+		if (this.initialValues_.hasOwnProperty(name) &&
+			!this.callValidator_(name, this.initialValues_[name])) {
+
 			delete this.initialValues_[name];
 		}
 	}

diff --git a/packages/metal-state/test/State.js b/packages/metal-state/test/State.js
@@ -307,6 +307,19 @@ describe('State', function() {
 		assert.strictEqual(1, state.key1);
 	});
 
+	it('should not overwrite default value with "undefined" initial value', function() {
+		var state = new State({
+			key1: undefined
+		});
+		state.configState({
+			key1: {
+				value: 'value1'
+			}
+		});
+
+		assert.equal(state.key1, 'value1');
+	});
+
 	it('should allow accessing other state properties in validator', function() {
 		var state = new State({
 			key1: 1