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

Send notifications on next tick #84

Merged
merged 3 commits into from Aug 26, 2020
Merged

Conversation

jstarry
Copy link
Contributor

@jstarry jstarry commented Aug 26, 2020

Fixes: #83

This is a quick and dirty fix for the race condition. Feel free to go with another approach

@mkozjak
Copy link
Member

mkozjak commented Aug 26, 2020

@jstarry thanks, looks good! I'm thinking we should add a nextTick library that can be used across platforms (node.js, web).
When it comes to node.js, process.nextTick is pretty much more performant than a js setTimeout.

https://github.com/medikoo/next-tick

@mkozjak mkozjak self-requested a review August 26, 2020 04:59
mkozjak added a commit to mkozjak/rpc-websockets that referenced this pull request Aug 26, 2020
Signed-off-by: Mario Kozjak <kozjakm1@gmail.com>
Signed-off-by: Mario Kozjak <kozjakm1@gmail.com>
@jstarry
Copy link
Contributor Author

jstarry commented Aug 26, 2020

Yes, good call. Want me to close this PR? Your branch looks good to me

@mkozjak
Copy link
Member

mkozjak commented Aug 26, 2020

@jstarry - no, just merge my pull request in your fork and I’ll merge this PR.

Use medikoo/next-tick over js' setTimeout
@jstarry
Copy link
Contributor Author

jstarry commented Aug 26, 2020

@mkozjak done. Do you mind pushing out a patch release for this fix? Not a big rush, but would be nice

@mkozjak mkozjak merged commit f7cea53 into elpheria:master Aug 26, 2020
@jstarry jstarry deleted the next-tick branch August 26, 2020 07:50
@mkozjak
Copy link
Member

mkozjak commented Aug 26, 2020

@jstarry: thanks for the PR - the feature is available in 7.1.0!

@jstarry
Copy link
Contributor Author

jstarry commented Aug 26, 2020

@mkozjak thanks for the quick turn around!

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

Successfully merging this pull request may close these issues.

Race condition in client RPC response queue handling
2 participants