-
Notifications
You must be signed in to change notification settings - Fork 607
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
Would you please provide a sync version of parseString #379
Comments
While what I find I am wrong is that the "async" version is not async at all. Strange behavior, indeed. I am going to close this issue,
|
I agree, but you can make the function async by setting the `async` param. The not-really-async behaviour was reported before but when I fixed it, much code broke becaus it just assumed that the function would be sync at heart, so I had to make it opt-in.
|
very confused! |
Why are you very confused? I can attempt to explain my reasoning in other words it that clears it up for you.
|
@Leonidas-from-XIV I think it's confused because I have to told everyone of my colleagues that the function is SYNC actually……anyway, It won't be a big problem~ |
@zszszsz so the right way to use this lib is: let parsed;
parseString(msgBody.message, (err, rst) => {
parsed = rst;
});
console.log(`${parsed}`); BTW It did take me some time because in my mind I think everything is OK with await when I saw the callback function. |
No, the right way is to use the callback and not exploit the fact that the callback is sync in the background which is an implementation detail and not guaranteed to stay working.
|
@Leonidas-from-XIV |
@sirius1024 or maybe you are also interested in #380. Anyway this issue is a closed one with proper solution. |
It is strange to provide only async functions without providing sync versions of them. :-)
The text was updated successfully, but these errors were encountered: