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

Flip the default value of allowSynchronousEvents #2221

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions doc/ws.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ This class represents a WebSocket server. It extends the `EventEmitter`.
in response to a ping. Defaults to `true`.
- `allowSynchronousEvents` {Boolean} Specifies whether any of the `'message'`,
`'ping'`, and `'pong'` events can be emitted multiple times in the same
tick. To improve compatibility with the WHATWG standard, the default value
is `false`. Setting it to `true` improves performance slightly.
tick. Defaults to `true`. Setting it to `false` improves compatibility with
the WHATWG standardbut may negatively impact performance.
- `backlog` {Number} The maximum length of the queue of pending connections.
- `clientTracking` {Boolean} Specifies whether or not to track clients.
- `handleProtocols` {Function} A function which can be used to handle the
Expand Down Expand Up @@ -302,8 +302,8 @@ This class represents a WebSocket. It extends the `EventEmitter`.
in response to a ping. Defaults to `true`.
- `allowSynchronousEvents` {Boolean} Specifies whether any of the `'message'`,
`'ping'`, and `'pong'` events can be emitted multiple times in the same
tick. To improve compatibility with the WHATWG standard, the default value
is `false`. Setting it to `true` improves performance slightly.
tick. Defaults to `true`. Setting it to `false` improves compatibility with
the WHATWG standardbut may negatively impact performance.
- `finishRequest` {Function} A function which can be used to customize the
headers of each HTTP request before it is sent. See description below.
- `followRedirects` {Boolean} Whether or not to follow redirects. Defaults to
Expand Down
7 changes: 5 additions & 2 deletions lib/receiver.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Receiver extends Writable {
* Creates a Receiver instance.
*
* @param {Object} [options] Options object
* @param {Boolean} [options.allowSynchronousEvents=false] Specifies whether
* @param {Boolean} [options.allowSynchronousEvents=true] Specifies whether
* any of the `'message'`, `'ping'`, and `'pong'` events can be emitted
* multiple times in the same tick
* @param {String} [options.binaryType=nodebuffer] The type for binary data
Expand All @@ -47,7 +47,10 @@ class Receiver extends Writable {
constructor(options = {}) {
super();

this._allowSynchronousEvents = !!options.allowSynchronousEvents;
this._allowSynchronousEvents =
options.allowSynchronousEvents !== undefined
? options.allowSynchronousEvents
: true;
this._binaryType = options.binaryType || BINARY_TYPES[0];
this._extensions = options.extensions || {};
this._isServer = !!options.isServer;
Expand Down
4 changes: 2 additions & 2 deletions lib/websocket-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class WebSocketServer extends EventEmitter {
* Create a `WebSocketServer` instance.
*
* @param {Object} options Configuration options
* @param {Boolean} [options.allowSynchronousEvents=false] Specifies whether
* @param {Boolean} [options.allowSynchronousEvents=true] Specifies whether
* any of the `'message'`, `'ping'`, and `'pong'` events can be emitted
* multiple times in the same tick
* @param {Boolean} [options.autoPong=true] Specifies whether or not to
Expand Down Expand Up @@ -60,7 +60,7 @@ class WebSocketServer extends EventEmitter {
super();

options = {
allowSynchronousEvents: false,
allowSynchronousEvents: true,
autoPong: true,
maxPayload: 100 * 1024 * 1024,
skipUTF8Validation: false,
Expand Down
4 changes: 2 additions & 2 deletions lib/websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ module.exports = WebSocket;
* @param {(String|URL)} address The URL to which to connect
* @param {Array} protocols The subprotocols
* @param {Object} [options] Connection options
* @param {Boolean} [options.allowSynchronousEvents=false] Specifies whether any
* @param {Boolean} [options.allowSynchronousEvents=true] Specifies whether any
* of the `'message'`, `'ping'`, and `'pong'` events can be emitted multiple
* times in the same tick
* @param {Boolean} [options.autoPong=true] Specifies whether or not to
Expand Down Expand Up @@ -652,7 +652,7 @@ module.exports = WebSocket;
*/
function initAsClient(websocket, address, protocols, options) {
const opts = {
allowSynchronousEvents: false,
allowSynchronousEvents: true,
autoPong: true,
protocolVersion: protocolVersions[1],
maxPayload: 100 * 1024 * 1024,
Expand Down
47 changes: 6 additions & 41 deletions test/receiver.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ describe('Receiver', () => {
buf[i + 1] = 0x00;
}

const receiver = new Receiver({ allowSynchronousEvents: true });
const receiver = new Receiver();
let counter = 0;

receiver.on('message', (data, isBinary) => {
Expand Down Expand Up @@ -1085,7 +1085,7 @@ describe('Receiver', () => {
receiver.write(Buffer.from([0x88, 0x03, 0x03, 0xe8, 0xf8]));
});

it('emits at most one event per event loop iteration', (done) => {
it('honors the `allowSynchronousEvents` option', (done) => {
const actual = [];
const expected = [
'1',
Expand Down Expand Up @@ -1121,7 +1121,7 @@ describe('Receiver', () => {
});
}

const receiver = new Receiver();
const receiver = new Receiver({ allowSynchronousEvents: false });

receiver.on('message', listener);
receiver.on('ping', listener);
Expand Down Expand Up @@ -1156,43 +1156,8 @@ describe('Receiver', () => {
done();
});

receiver.write(Buffer.from('82008200', 'hex'));
});

it('honors the `allowSynchronousEvents` option', (done) => {
const actual = [];
const expected = [
'1',
'2',
'3',
'4',
'microtask 1',
'microtask 2',
'microtask 3',
'microtask 4'
];

function listener(data) {
const message = data.toString();
actual.push(message);

// `queueMicrotask()` is not available in Node.js < 11.
Promise.resolve().then(() => {
actual.push(`microtask ${message}`);

if (actual.length === 8) {
assert.deepStrictEqual(actual, expected);
done();
}
});
}

const receiver = new Receiver({ allowSynchronousEvents: true });

receiver.on('message', listener);
receiver.on('ping', listener);
receiver.on('pong', listener);

receiver.write(Buffer.from('8101318901328a0133810134', 'hex'));
setImmediate(() => {
receiver.write(Buffer.from('82008200', 'hex'));
});
});
});