From 71596648c2c30e16cb7c94e73dbb3538beb06534 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 21 Oct 2019 17:52:01 -0700 Subject: [PATCH] typing_status: Switch sentinel "recipient" value to `null`. This feels a bit more semantically appropriate: it more clearly says "here's some information: there is no (relevant) recipient", rather than "no information available". (Both `null` and `undefined` in JS can have either meaning, but `undefined` especially commonly means the latter.) Concretely, it ensures a bit more explicitness where the value originates: a bare `return;` becomes `return null;`, reflecting the fact that it is returning a quite informative value. Also make the implementation more explicit about what's expected here, replacing truthiness tests with `!== null`. (A bit more idiomatic would be `!= null`, which is equivalent when the value is well-typed and a bit more robust to ill-typing bugs. But lint complains about that version.) --- frontend_tests/node_tests/typing_status.js | 10 +++++----- static/js/typing.js | 6 +++--- static/shared/js/typing_status.js | 16 ++++++++-------- static/shared/js/typing_status.js.flow | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/frontend_tests/node_tests/typing_status.js b/frontend_tests/node_tests/typing_status.js index 72bafc884d..0d75143082 100644 --- a/frontend_tests/node_tests/typing_status.js +++ b/frontend_tests/node_tests/typing_status.js @@ -16,7 +16,7 @@ run_test('basics', () => { // invalid conversation basically does nothing var worker = {}; - typing_status.update(worker, undefined); + typing_status.update(worker, null); // Start setting up more testing state. typing_status.initialize_state(); @@ -128,7 +128,7 @@ run_test('basics', () => { }); // Call stop with nothing going on. - call_handler(undefined); + call_handler(null); assert.deepEqual(typing_status.state, { next_send_start_time: undefined, idle_timer: undefined, @@ -158,7 +158,7 @@ run_test('basics', () => { assert(events.idle_callback); // Explicitly stop alice. - call_handler(undefined); + call_handler(null); assert.deepEqual(typing_status.state, { next_send_start_time: undefined, idle_timer: undefined, @@ -188,7 +188,7 @@ run_test('basics', () => { assert(events.idle_callback); // Switch to an invalid conversation. - call_handler(undefined); + call_handler(null); assert.deepEqual(typing_status.state, { next_send_start_time: undefined, idle_timer: undefined, @@ -202,7 +202,7 @@ run_test('basics', () => { }); // Switch to another invalid conversation. - call_handler(undefined); + call_handler(null); assert.deepEqual(typing_status.state, { next_send_start_time: undefined, idle_timer: undefined, diff --git a/static/js/typing.js b/static/js/typing.js index 333fa419d8..4d01809bb5 100644 --- a/static/js/typing.js +++ b/static/js/typing.js @@ -26,7 +26,7 @@ function send_typing_notification_ajax(user_ids_array, operation) { function get_user_ids_array() { var user_ids_string = compose_pm_pill.get_user_ids_string(); if (user_ids_string === "") { - return; + return null; } return people.user_ids_string_to_ids_array(user_ids_string); @@ -65,14 +65,14 @@ exports.initialize = function () { // If our previous state was no typing notification, send a // start-typing notice immediately. var new_recipient = - is_valid_conversation() ? exports.get_recipient() : undefined; + is_valid_conversation() ? exports.get_recipient() : null; typing_status.update(worker, new_recipient); }); // We send a stop-typing notification immediately when compose is // closed/cancelled $(document).on('compose_canceled.zulip compose_finished.zulip', function () { - typing_status.update(worker, undefined); + typing_status.update(worker, null); }); }; diff --git a/static/shared/js/typing_status.js b/static/shared/js/typing_status.js index e3121574c3..066996751e 100644 --- a/static/shared/js/typing_status.js +++ b/static/shared/js/typing_status.js @@ -17,8 +17,8 @@ export const state = {}; /** Exported only for tests. */ export function initialize_state() { - state.current_recipient = undefined; - state.next_send_start_time = undefined; + state.current_recipient = null; + state.next_send_start_time = undefined; state.idle_timer = undefined; } @@ -81,9 +81,9 @@ export function maybe_ping_server(worker, recipient) { * composing a stream message should be treated like composing no message at * all. * - * Call with `new_recipient` of `undefined` when the user actively stops + * Call with `new_recipient` of `null` when the user actively stops * composing a message. If the user switches from one set of recipients to - * another, there's no need to call with `undefined` in between; the + * another, there's no need to call with `null` in between; the * implementation tracks the change and behaves appropriately. * * See docs/subsystems/typing-indicators.md for detailed background on the @@ -92,12 +92,12 @@ export function maybe_ping_server(worker, recipient) { * @param {*} worker Callbacks for reaching the real world. See typing.js * for implementations. * @param {*} new_recipient The users the PM being composed is addressed to, - * as a sorted array of user IDs; or `undefined` if no PM is being - * composed anymore. + * as a sorted array of user IDs; or `null` if no PM is being composed + * anymore. */ export function update(worker, new_recipient) { var current_recipient = state.current_recipient; - if (current_recipient) { + if (current_recipient !== null) { // We need to use _.isEqual for comparisons; === doesn't work // on arrays. if (_.isEqual(new_recipient, current_recipient)) { @@ -117,7 +117,7 @@ export function update(worker, new_recipient) { stop_last_notification(worker); } - if (!new_recipient) { + if (new_recipient === null) { // If we are not talking to somebody we care about, // then there is no more action to take. return; diff --git a/static/shared/js/typing_status.js.flow b/static/shared/js/typing_status.js.flow index 21935d0a49..1bd54be473 100644 --- a/static/shared/js/typing_status.js.flow +++ b/static/shared/js/typing_status.js.flow @@ -14,5 +14,5 @@ type Worker = {| declare export function update( worker: Worker, - new_recipient: RecipientUserIds | void, + new_recipient: RecipientUserIds | null ): void;