Skip to content

Conversation

@Borewit
Copy link

@Borewit Borewit commented Feb 2, 2025

While posting answer on StackOverflow using mocket-socket, I found a few issues in the sample code.

Changes:

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));
Copy link
Collaborator

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');

Copy link
Author

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.

@Atrue
Copy link
Collaborator

Atrue commented Feb 7, 2025

@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`
@Borewit Borewit force-pushed the readme-fix-code-example branch from 5eff5f5 to 78adba6 Compare February 7, 2025 14:42
@Borewit
Copy link
Author

Borewit commented Feb 7, 2025

@Borewit Sorry for the long delay. Can you please fix the same for the socket.io part of the README?

Sorry, I will keep with PR with that example. To many things on my plate.

```js
import test from 'ava';
import { Server } from 'mock-socket';
import {AsyncQueue} from '@borewit/async-queue';
Copy link
Collaborator

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

Copy link
Author

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.

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.

2 participants