Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible fix for the error asserting issue #211

Closed
wants to merge 2 commits into from

Conversation

ocrest
Copy link

@ocrest ocrest commented Aug 10, 2017

Hi,

This pull-request is just an idea (maybe crazy) which perhaps solve the issue emberjs/ember.js#15013

The bottom line is this: we can set a global window.onerror handler before the component render, and save all uncaught errors (test-module-for-component.js#L243-L247).

Then, after the component was rendered, we rethrow these errors to be caught by the test framework error assertion: test-module-for-component.js#L258-L263

I created a simple test with this component:

// A component which throws an error during render:
var BadComponent = Ember.Component.extend({
  layout: Ember.Handlebars.compile('<input type="text" onfocus={{action "focus"}}>'),
  didInsertElement() {
    this.set('input', this.element.querySelector('input'));
  },
  didRender() {
    this.get('input').focus();
  },
  actions: {
    focus() {
      this.get('unexistedMethod')();
    }
  }
});

— it sets the focus to the child <input> element in the didRender hook, and this triggers the focus() closure action, which, in turn, throws an error (because there is a call on the undefined property).

With this hacky fix the throws assertion

test('it correctly handles uncaught errors when rendering a component', function() {
  var self = this;
  throws(function() {
    self.render('{{bad-component}}');
  });
});

now successfully catches this error.

Any thoughts?

@rwjblue
Copy link
Member

rwjblue commented Nov 28, 2017

The underlying issue reported in emberjs/ember.js#15013 was fixed by emberjs/ember.js#15871, which essentially reverts emberjs/ember.js#14898 and adds a bunch of tests.

Closing...

@rwjblue rwjblue closed this Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants