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

Ensure RSVP promises do not need manual Ember.run wrapping. #201

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Apr 3, 2017

This is essentially doing the same thing that was added way back in Ember 1.6/1.7 by Stef.

The basic gist is that whenever something is triggering RSVP's async it will wrap that in a Ember.run. This prevents folks from getting the autorun error for just resolving promises...

This brings the ember-metal-test-friendly-promises feature (from emberjs/ember.js#4176) to ember-test-helpers.

This is essentially doing the same thing that was added way back
in Ember 1.6/1.7 by Stef.

The basic gist is that whenever something is triggering RSVP's
`async` it will wrap that in a `Ember.run`. This prevents folks
from getting the autorun error for just resolving promises...
@cibernox
Copy link
Contributor

cibernox commented Apr 4, 2017

LGTM. Do u want me to test it locally on ember-native-dom-helpers?

Copy link
Member

@trentmwillis trentmwillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. The changing of RSVP's async always makes me wary, but in this case I think it should be beneficial.

@rwjblue
Copy link
Member Author

rwjblue commented Apr 4, 2017

FWIW, this mirrors what is done in ember's ember-testing package for acceptance tests now. The super long term goal is to slowly decouple from ember-testing...

@rwjblue
Copy link
Member Author

rwjblue commented Apr 4, 2017

@cibernox - I do not think this will result in any changes to ember-native-dom-helpers actually...

@rwjblue rwjblue merged commit 4bbb53c into emberjs:master Apr 4, 2017
@rwjblue rwjblue deleted the auto-run-wrap-promises branch April 4, 2017 15:44
@cibernox
Copy link
Contributor

cibernox commented Apr 4, 2017

@rwjblue I understood that this was making wait() wait until all promises are resolved, did I understand the PR wrong?

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.

4 participants