Skip to content

Conversation

schrockn-zz
Copy link
Contributor

This PR adds the async/await babel polyfill and then changes src/tests (the Star Wars tests) to use them. Using this transform makes life a lot easier.

  1. The syntax is nicer
  2. No need to use the promise-friendly chai from the chai-as-promised module. I dislike these directives because it makes it awkward to add console logging in a test in between the executed test code and then test of the result. I also think its often desirable to do multiple expects with more granular error messaging.
  3. No more forgetting to return the promise and then have false-passes because the code just doesn't execute.
  4. From brief testing the errors messages appear more reasonable.

I think we should just get rid of chai-as-promised and switch to this.

@leebyron
Copy link
Contributor

Sweeeeeeet.

Are the changes to package.json necessary? As long as the async/await is only used within tests they might not be. Only curious because of concern that babel might include more polyfills that aren't being used in the built and npm published module

@leebyron
Copy link
Contributor

How are you seeing error messages change? Exciting to hear that you're seeing clearer messages, I'm curious what that looks like

@schrockn-zz
Copy link
Contributor Author

It doesn't appear so. I tested npm run build with --ignore tests commented out and it worked.

@schrockn-zz
Copy link
Contributor Author

Lee I might be confusing it with your change but I can look into that more carefully. One sec.

@leebyron
Copy link
Contributor

Also, I agree that this syntax is nicer than chai as promised. I would love to see this pattern replace that dependency

@leebyron
Copy link
Contributor

Also make sure npm run cover works as well. That might be the thing in package.json that actually might require a change

@schrockn-zz
Copy link
Contributor Author

Yeah I think I got confused there on the error reporting.

@schrockn-zz
Copy link
Contributor Author

There are no magical flags in package.json. Do you mean .eslintrc?

schrockn-zz added a commit that referenced this pull request Jul 16, 2015
@schrockn-zz schrockn-zz merged commit 91e6109 into master Jul 16, 2015
@leebyron leebyron deleted the babel-async-await branch July 16, 2015 06:27
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