diff --git a/packages/metal-state/src/State.js b/packages/metal-state/src/State.js index b621648f..59b4a424 100644 --- a/packages/metal-state/src/State.js +++ b/packages/metal-state/src/State.js @@ -382,10 +382,10 @@ class State extends EventEmitter { /** * Returns an array with all state keys. - * @return {Array.} + * @return {!Array.} */ getStateKeys() { - return Object.keys(this.stateInfo_); + return this.stateInfo_ ? Object.keys(this.stateInfo_) : []; } /** @@ -396,8 +396,10 @@ class State extends EventEmitter { * @protected */ getStateKeyValue_(name) { - this.initStateKey_(name); - return this.stateInfo_[name].value; + if (!this.warnIfDisposed_(name)) { + this.initStateKey_(name); + return this.stateInfo_[name].value; + } } /** @@ -411,7 +413,7 @@ class State extends EventEmitter { return info.state === State.KeyStates.INITIALIZED || this.hasInitialValue_(name); } - + /** * Checks if an initial value was given to the specified state property. * @param {string} name The name of the key. @@ -428,7 +430,9 @@ class State extends EventEmitter { * @return {boolean} */ hasStateKey(key) { - return !!this.stateInfo_[key]; + if (!this.warnIfDisposed_(key)) { + return !!this.stateInfo_[key]; + } } /** @@ -612,7 +616,9 @@ class State extends EventEmitter { * @protected */ setStateKeyValue_(name, value) { - if (!this.canSetState(name) || !this.validateKeyValue_(name, value)) { + if (this.warnIfDisposed_(name) || + !this.canSetState(name) || + !this.validateKeyValue_(name, value)) { return; } @@ -660,6 +666,22 @@ class State extends EventEmitter { return info.state === State.KeyStates.INITIALIZING || this.callValidator_(name, value); } + + /** + * Warns if this instance has already been disposed. + * @param {string} name Name of the property to be accessed if not disposed. + * @return {boolean} True if disposed, or false otherwise. + * @protected + */ + warnIfDisposed_(name) { + const disposed = this.isDisposed(); + if (disposed) { + console.warn( + `Error. Trying to access property "${name}" on disposed instance` + ); + } + return disposed; + } } /** diff --git a/packages/metal-state/test/State.js b/packages/metal-state/test/State.js index cb6893a5..a75ac09d 100644 --- a/packages/metal-state/test/State.js +++ b/packages/metal-state/test/State.js @@ -412,7 +412,7 @@ describe('State', function() { } } }; - + new Test({ key1: undefined }); @@ -713,16 +713,6 @@ describe('State', function() { state.key2 = 21; }); - it('should not throw error when trying to emit scheduled stateChanged after disposed', function(done) { - var state = createStateInstance(); - - state.key1 = 10; - state.dispose(); - async.nextTick(function() { - done(); - }); - }); - it('should call callback function from setState asynchronously after the batch event is triggered', function(done) { var state = createStateInstance(); @@ -816,14 +806,6 @@ describe('State', function() { assert.strictEqual(0, listener.callCount); }); - it('should not allow getting state data after disposed', function() { - var state = createStateInstance(); - state.dispose(); - assert.throws(function() { - state.getState(); - }); - }); - describe('Static STATE', function() { function createTestClass() { class Test extends State { @@ -1083,6 +1065,59 @@ describe('State', function() { assert.strictEqual(2, listener.callCount); }); }); + + describe('Dispose', function() { + const originalWarnFn = console.warn; + + beforeEach(function() { + console.warn = sinon.stub(); + }); + + afterEach(function() { + console.warn = originalWarnFn; + }); + + it('should not throw error when trying to emit scheduled stateChanged after disposed', function(done) { + var state = createStateInstance(); + + state.key1 = 10; + state.dispose(); + async.nextTick(function() { + done(); + }); + }); + + it('should warn if trying to get state property after disposed', function() { + var state = createStateInstance(); + state.dispose(); + assert.doesNotThrow(() => state.key1); + assert.strictEqual(1, console.warn.callCount); + }); + + it('should warn if trying to set state property after disposed', function() { + var state = createStateInstance(); + state.dispose(); + assert.doesNotThrow(() => state.key1 = 'new'); + assert.strictEqual(1, console.warn.callCount); + }); + + it('should warn if trying to use "setState" after disposed', function() { + var state = createStateInstance(); + state.dispose(); + assert.doesNotThrow(() => { + state.setState({ + key1: 'new' + }); + }); + assert.strictEqual(1, console.warn.callCount); + }); + + it('should not throw error if trying to use "getState" after disposed', function() { + var state = createStateInstance(); + state.dispose(); + assert.doesNotThrow(() => state.getState()); + }); + }); }); function createStateInstance() {