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

Unit tests for async/await (ES2017) usage #192

Open
oberstet opened this issue Apr 28, 2016 · 7 comments
Open

Unit tests for async/await (ES2017) usage #192

oberstet opened this issue Apr 28, 2016 · 7 comments
Assignees
Labels
CI-CD Test, build and packaging infra core enhancement

Comments

@oberstet
Copy link
Contributor Author

This should also work on non-ES7 browsers/node via:

'fast-async' is a Babel v6.x.x plugin that implements the ES7 keywords async and await using syntax transformation at compile-time rather than generators.

@MakuraYami
Copy link

NodeJS has the option --harmony-async-await since version 7, another method is using typescript which since recently supports async/await.

Talking about typescript. The types published on npm @types/autobahn are horrible and force everything as a When promise while in node. You're way better off using regular promises. Which typescript actually enforces when using async methods. I had to edit my types locally to get it to compile without errors.

More support on the newest features in autobahn and removal of the When dependency in node should be a priority in my opinion. It won't be long before edge, chrome and Firefox all support async/await in the stable builds.

@oberstet
Copy link
Contributor Author

You're way better off using regular promises

We need to support older browsers ..

@MakuraYami
Copy link

https://github.com/taylorhakes/promise-polyfill

Is that really still an excuse? Use a polyfill for browsers, don't bother nodejs users with it.

@oberstet
Copy link
Contributor Author

oberstet commented Apr 20, 2017

@MakuraYami No, it is no excuse;) You are right, I agree. In fact, we need more tests over the current JS API with callbacks and promises, then more tests over an await/async style usage of the library, as well as tests over the library API surface when used from Typescript, eg this

@oberstet oberstet changed the title Test and document using async/await (ES2017) with AutobahnJS Unit tests for async/await (ES2017) usage Apr 20, 2017
@oberstet
Copy link
Contributor Author

You're way better off using regular promises. Which typescript actually enforces when using async methods.

@MakuraYami So what's your opinion about #260? Should we add definitions for Typescript in this repo or in DefinitelyTyped? Would you like to propose a PR based on the hacked version you have?

@MakuraYami
Copy link

MakuraYami commented Apr 21, 2017

Hi @oberstet, Thanks for your input and work on the project and these topics. In my opinion DefinitelyTyped is a place where people can set up types for libraries that do not support these first hand, but it requires an additional dependency "@types/autobahn". When you add index.d.ts to the autobahn repository typescript will automatically use that one above the one in node_modules.

If you are in favor of supporting typescript I would take the file from DefinitelyTyped and move it to the autobahn repository, as for the edits I made it was only changing When.Promise into PromiseLike so that it would no longer error when I made it use native promises instead of When.

Promise libraries still play a good role in JavaScript development, if they offer useful additional functionality like bluebird does. But if it is just a poly-fill I really think it should be treated like one.

async/await is nice, but to be honest the code base is not large enough to really worry about it. I would be in favor of just switching to the native promsie and use that, being able to pass your own promise method is still nice though, for when you would want to use bluebird instead. If some day you write the library in TypeScript entirely then you can easily use async/await, as the compiler will make it work everywhere.

So to summarize I would recommend moving the type definition into the repository and removing the When dependency. That would make working with autobahn-js a lot more pleasant 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-CD Test, build and packaging infra core enhancement
Projects
None yet
Development

No branches or pull requests

4 participants