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

Better compatibility with async/await function in test #175

Closed
GCorbel opened this issue Nov 1, 2016 · 15 comments
Closed

Better compatibility with async/await function in test #175

GCorbel opened this issue Nov 1, 2016 · 15 comments

Comments

@GCorbel
Copy link

GCorbel commented Nov 1, 2016

Hi,

This issue is related to this other one.

I like the next 'async/await' feature proposed in es7. I saw in this article than it can works but I don't understand why because it will automatically fail if Ember.testing is true as you can see on this line.

My main goal is to have a cleaner syntax. I tried to do an action like this :

  actions: {
    async clone(model) {
      const clone = await model.clone();

      clone.set('name', clone.get('name') + ' - copie');
      clone.set('activity', model.get('activity'));
      this._nestedSave(clone, 'groups');
    }
  }

It works but I cannot test it because I have the message "You have turned on testing mode, which disabled the run-loop's autorun. You will need to wrap any code with asynchronous side-effects in a run". I described the error in this SO question. I know I can use Ember.run but async/await become useless.

I saw I can set Ember.testing to false but there will be problems later.

I think a better way to manage async/await feature is needed.

What you think?

@rwjblue
Copy link
Member

rwjblue commented Nov 1, 2016

I think a better way to manage async/await feature is needed.

I don't think this is related to async/await really, the main issue is with the set that is done outside of the run loop. Writing the same code in a promise style would have the same issue.

To me the real question is: can we avoid requiring Ember.set / this.set to be wrapped in a run loop? Right now the answer is no, but it is something that we have discussed at length and would like to find a path towards.

@dfreeman
Copy link
Contributor

dfreeman commented Nov 1, 2016

Writing the same code in a promise style would have the same issue.

I was under the impression RSVP would create a run loop (even in testing mode) when executing handlers after one of its promises settled. Is that a bad assumption?

https://ember-twiddle.com/e2721848f818029dd39873e3da88870c?openFiles=tests.unit.runloop-test.js%2C

@GCorbel
Copy link
Author

GCorbel commented Nov 1, 2016

I don't have any problem with the same code in promise version. Here is the code :

    clone(model) {
      model.clone().then(clone=> {
        clone.set('name', clone.get('name') + ' - copie');
        clone.set('activity', model.get('activity'));
        this._nestedSave(clone, 'groups');
      });
    }

@GCorbel
Copy link
Author

GCorbel commented Nov 30, 2016

I have to investigate a little bit more because it seems to work on another project. My function model.clone use promises and I have to check if I can replace it by async/await functions.

@topaxi
Copy link

topaxi commented Nov 30, 2016

Regenerator uses the global Promise object, this means you need to replace window.Promise with RSVP.Promise somewhere before any async function gets executed.

Example:
app/initializers/promise.js

import RSVP from 'rsvp'

export default {
  name: 'promise',
  initialize() {
    window.Promise = RSVP.Promise
  }
}

This probably won't work anymore once native async functions land though.

@GCorbel
Copy link
Author

GCorbel commented Nov 30, 2016

@topaxi I think you're right. It works with your initializer. I will do more tests.

@GCorbel
Copy link
Author

GCorbel commented Nov 30, 2016

There is an issue with ember-cli-page-object. In a test, I have await page.submit(); and it doesn't work but await click('#submit');. It's probably because of this line.

What I can see is wait callback is not triggered. For example, with :

wait(()=> {
  console.log(1);
});

I can't show "1" in the console. wait is waiting for the last promise. I don't understand why it doesn't work.

@jrjohnson
Copy link
Sponsor

This bit me while attempting to build an async computed property like:

foo: computed('bar', async function(){
  return await this.get('bar');
})

It works surprisingly well and renders great, but breaks any tests with the "wrap any code with asynchronous side-effects" error.

Adopting @topaxi's solution of replacing the global Promise with RSVP.Promise solves the issue. I assume RSVP.Promise has some magic to notify qunit of asynchronous work to come?

This would seem to be the kind of issue that needs to be addressed at the framework level before the use of async / await picks up momentum. I've seen several blog posts advocating its use with Ember already; most recently https://spin.atomicobject.com/2016/12/29/async-await-ember-project/ which was listed in Ember Weekly a few weeks ago.

@theseyi
Copy link

theseyi commented Jun 14, 2017

Hey @rwjblue any potential updates on this issue without resorting to overriding the global Promise object per @topaxi 's solution? Teammate just encountered a similar scenario. Running the snippet below in an async/await raises the same exception, but changing it to a Promise based func, results in the test executing as expected.

promise: works

 authenticateUser() {
      const { username, password, authenticator } = getProperties(this, ['username', 'password', 'authenticator']);
      const setAuthFlag = () => {
        if (!(this.isDestroying || this.isDestroyed)) {
          set(this, isAuthenticatingFlag, false);
        }
      }

      // Set the isAuthenticatingFlag on the host object, and default error message string to empty
      setProperties(this, { [isAuthenticatingFlag]: true, errorMessage: '' });

      return get(this, 'session').authenticate('authenticator', { username, password }).then(() => {
        setAuthFlag();
      }).catch(({ error = errorMessage }) => {
        set(this, 'errorMessage', error);
        setAuthFlag();
      });
    }
  }

versus:
async/await: does not work

    async authenticateUser() {
      const { username, password, authenticator } = getProperties(this, ['username', 'password', 'authenticator']);
      let result;
      // Set the isAuthenticatingFlag on the host object, and default error message string to empty
      setProperties(this, { [isAuthenticatingFlag]: true, errorMessage: '' });

      try {
        // Send the username and password to the specified Ember Auth authenticator
        result = await get(this, 'session').authenticate('authenticator', { username, password });
      } catch ({ error = errorMessage }) {
        set(this, 'errorMessage', error);
      } finally {
        if (!(this.isDestroying || this.isDestroyed)) {
          set(this, isAuthenticatingFlag, false);
        }
      }

      return result;
    }

@caseywatts
Copy link

caseywatts commented Aug 31, 2017

I've been trying to use async/await in both app code and test code this week, and I ran into this issue too.

I think I'm going to avoid using async/await in app code for now. I definitely don't want to type Ember.run everywhere, and patching window.Promise to use the RSVP version (which includes the Ember.run) would only be a ~short-term fix. It does work currently on a few promises in our app that aren't Ember-y, but it's hard to keep track of which are which so we might just abandon using async/await in app code generally.

I could maybe imagine a parallel version of await, called something similar (like awaiit?) that acts like await but also calls the Ember.run loop. Just like RSVP does for Promises. But I'm not sure how useful that would be unless we could get it to behave like await inline too, like in const thing = await asyncThing();,

Async/await still helps a lot in tests, though! :)

I have some more notes on this, here: https://gist.github.com/caseywatts/7c01654f74fbe402dfaadfa144965adb

@sukima
Copy link

sukima commented Sep 15, 2017

Thank you @caseywatts I just got bit with this today. Thank you for commenting! This issue of async/await is the core reason for my RFC to ember-concurrency concerning exposing a coroutine API.

Sadly neither async/await nor a coroutine exposed from e-c is in the works. I think the next step would be to make an addon that wraps tj/co.

@sukima
Copy link

sukima commented Sep 16, 2017

For a work around there is an addon: https://www.npmjs.com/package/ember-co

@sukima
Copy link

sukima commented May 31, 2018

The discuss discussion here seems more relevant now; suggesting that this issue is fixed in Ember 3.0+

Can anyone confirm?

@dfreeman
Copy link
Contributor

I believe that post suggests that the issue can be fixed in 3.0+, but that so far BackburnerJS/backburner.js#332 has struck any time someone has attempted it.

@rwjblue
Copy link
Member

rwjblue commented May 30, 2019

Ember 3.4 has dropped the auto run assertion, and using async functions in "production" code is quite feasible. There are still some rough edges, but overall it works very well.

Closing this now, as "implemented".

@rwjblue rwjblue closed this as completed May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants