From ab4a2e367f042e7b82cb62d740e9bf03637a1d62 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 14 Feb 2018 14:34:31 -0500 Subject: [PATCH] [BUGFIX lts] Ensure errors in deferred component hooks can be recovered. Previously, any errors thrown during `didInsertElement` would leave the running environment in an invalid state (`env.inTransaction` would be `true` but `this._transaction` would have been nullified). This commit ensures that we _always_ reset `inTransaction` if `Environment.prototype.commit` is called. Thus avoiding an error RE: "calling commit on null"... --- packages/ember-glimmer/lib/environment.ts | 8 ++- .../components/error-handling-test.js | 55 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/packages/ember-glimmer/lib/environment.ts b/packages/ember-glimmer/lib/environment.ts index 78abd10c54d..47371d9ebcf 100644 --- a/packages/ember-glimmer/lib/environment.ts +++ b/packages/ember-glimmer/lib/environment.ts @@ -207,9 +207,11 @@ export default class Environment extends GlimmerEnvironment { destroyedComponents[i].destroy(); } - super.commit(); - - this.inTransaction = false; + try { + super.commit(); + } finally { + this.inTransaction = false; + } } } diff --git a/packages/ember-glimmer/tests/integration/components/error-handling-test.js b/packages/ember-glimmer/tests/integration/components/error-handling-test.js index 9614b74cca6..f35d35ab56e 100644 --- a/packages/ember-glimmer/tests/integration/components/error-handling-test.js +++ b/packages/ember-glimmer/tests/integration/components/error-handling-test.js @@ -69,4 +69,59 @@ moduleFor('Errors thrown during render', class extends RenderingTest { this.assertText('hello'); } + + ['@test it can recover resets the transaction when an error is thrown during didInsertElement'](assert) { + let shouldThrow = true; + let FooBarComponent = Component.extend({ + didInsertElement() { + this._super(...arguments); + if (shouldThrow) { + throw new Error('silly mistake!'); + } + } + }); + + this.registerComponent('foo-bar', { ComponentClass: FooBarComponent, template: 'hello' }); + + assert.throws(() => { + this.render('{{#if switch}}{{#foo-bar}}{{foo-bar}}{{/foo-bar}}{{/if}}', { switch: true }); + }, /silly mistake/); + + assert.equal(this.env.inTransaction, false, 'should not be in a transaction even though an error was thrown'); + + this.assertText('hello'); + + this.runTask(() => set(this.context, 'switch', false)); + + this.assertText(''); + } + + ['@test it can recover resets the transaction when an error is thrown during destroy'](assert) { + let shouldThrow = true; + let FooBarComponent = Component.extend({ + destroy() { + this._super(...arguments); + if (shouldThrow) { + throw new Error('silly mistake!'); + } + } + }); + + this.registerComponent('foo-bar', { ComponentClass: FooBarComponent, template: 'hello' }); + + this.render('{{#if switch}}{{#foo-bar}}{{foo-bar}}{{/foo-bar}}{{/if}}', { switch: true }); + + this.assertText('hello'); + + assert.throws(() => { + this.runTask(() => set(this.context, 'switch', false)); + }, /silly mistake/); + + this.assertText(''); + + shouldThrow = false; + this.runTask(() => set(this.context, 'switch', true)); + + this.assertText('hello'); + } });