crossbario / autobahn-js

WAMP in JavaScript for Browsers and NodeJS
http://crossbar.io/autobahn
MIT License
1.43k stars 228 forks source link

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

Open oberstet opened 8 years ago

oberstet commented 8 years ago

In principle, this should work already (eg on Edge/Chakra)

oberstet commented 8 years ago

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 commented 7 years ago

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 commented 7 years ago

You're way better off using regular promises

We need to support older browsers ..

MakuraYami commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 :+1: