-
Notifications
You must be signed in to change notification settings - Fork 121
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
Fix code example #393
base: master
Are you sure you want to change the base?
Fix code example #393
Conversation
README.md
Outdated
t.is(app.messages[0], 'test message from mock server', 'we have stubbed our websocket backend'); | ||
mockServer.stop(t.done); | ||
}, 100); | ||
await new Promise(resolve => setTimeout(resolve, 100)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to avoid using setTimeout
at all. I think it's worth adding some helpers.
const waitFor = (connection, event) => new Promise(resolve => {
const handler = () => {
resolve();
connection.removeEventListener(event, handler);
}
connection.addEventListener(event, handler);
});
const app = new ChatApp(fakeURL);
await waitFor(app.connection, 'open');
...
await waitFor(app.connection, 'message');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The are quite some issues with suggestion provided, so I used an alternative approach.
@Borewit Sorry for the long delay. Can you please fix the same for the socket.io part of the README? |
Changes: - Fixes trying to emit a connection when the WebSocket is not yet connected. - Updated AVA test, resolving thoov#388 - Convert incoming messages to `AsyncQueue`
5eff5f5
to
78adba6
Compare
Sorry, I will keep with PR with that example. To many things on my plate. |
@@ -37,23 +37,35 @@ import { WebSocket, Server } from 'mock-socket'; | |||
```js | |||
import test from 'ava'; | |||
import { Server } from 'mock-socket'; | |||
import {AsyncQueue} from '@borewit/async-queue'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to keep the readme simple and avoid using 3rd party libs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully understand that. I tried to embed an inline solution, yet that started to a bit lengthy and distracting, hence I moved this portion to borewit/async-queue.
I fixed the code example in 3 different area's. I leave it as is.
While posting answer on StackOverflow using mocket-socket, I found a few issues in the sample code.
Changes: