From c7ab3884c6d2b0daba6dc6e00e0c5edd05635e69 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 4 Aug 2018 13:40:25 +0000 Subject: [PATCH] refactor: Extract reload_state module. This is part of work to break some of our nastier circular dependencies in preparation for our es6 migration. This commit should facilitate loading leaf-like modules such as people.js before all of the things that reload.js depends on. --- .eslintrc.json | 1 + frontend_tests/node_tests/activity.js | 6 +-- frontend_tests/node_tests/compose_actions.js | 2 +- frontend_tests/node_tests/people_errors.js | 4 +- frontend_tests/node_tests/presence.js | 2 +- frontend_tests/node_tests/server_events.js | 2 +- frontend_tests/node_tests/transmit.js | 3 +- static/js/activity.js | 2 +- static/js/bundles/app.js | 1 + static/js/compose_actions.js | 2 +- static/js/people.js | 2 +- static/js/presence.js | 2 +- static/js/reload.js | 21 +++------- static/js/reload_state.js | 43 ++++++++++++++++++++ static/js/server_events.js | 2 +- static/js/transmit.js | 2 +- 16 files changed, 66 insertions(+), 31 deletions(-) create mode 100644 static/js/reload_state.js diff --git a/.eslintrc.json b/.eslintrc.json index d03101f9ff..4aaf93f316 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -118,6 +118,7 @@ "realm_icon": false, "recent_senders": false, "reload": false, + "reload_state": false, "reminder": false, "resize": false, "rows": false, diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index 7bed6c1371..86662a8d5e 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -48,7 +48,7 @@ const _stream_popover = { }, }; -const _reload = { +const _reload_state = { is_in_progress: () => false, }; @@ -66,7 +66,7 @@ set_global('feature_flags', _feature_flags); set_global('keydown_util', _keydown_util); set_global('page_params', _page_params); set_global('popovers', _popovers); -set_global('reload', _reload); +set_global('reload_state', _reload_state); set_global('resize', _resize); set_global('scroll_util', _scroll_util); set_global('stream_popover', _stream_popover); @@ -853,7 +853,7 @@ run_test('initialize', () => { assert(!activity.has_focus); // Now execute the reload-in-progress code path - reload.is_in_progress = function () { + _reload_state.is_in_progress = function () { return true; }; diff --git a/frontend_tests/node_tests/compose_actions.js b/frontend_tests/node_tests/compose_actions.js index a043107370..eef6bed27c 100644 --- a/frontend_tests/node_tests/compose_actions.js +++ b/frontend_tests/node_tests/compose_actions.js @@ -44,7 +44,7 @@ compose_state.recipient = (function () { }; }()); -set_global('reload', { +set_global('reload_state', { is_in_progress: return_false, }); diff --git a/frontend_tests/node_tests/people_errors.js b/frontend_tests/node_tests/people_errors.js index c5fa3db950..027f726399 100644 --- a/frontend_tests/node_tests/people_errors.js +++ b/frontend_tests/node_tests/people_errors.js @@ -2,7 +2,7 @@ zrequire('people'); var return_false = function () { return false; }; var return_true = function () { return true; }; -set_global('reload', { +set_global('reload_state', { is_in_progress: return_false, }); @@ -27,7 +27,7 @@ run_test('report_late_add', () => { assert.equal(blueslip.get_test_logs('error').length, 1); blueslip.clear_test_data(); - reload.is_in_progress = return_true; + reload_state.is_in_progress = return_true; people.report_late_add(55, 'foo@example.com'); assert.equal(blueslip.get_test_logs('log').length, 1); assert.equal(blueslip.get_test_logs('log')[0].message, 'Added user late: user_id=55 email=foo@example.com'); diff --git a/frontend_tests/node_tests/presence.js b/frontend_tests/node_tests/presence.js index baa5beaa68..3310ca25a9 100644 --- a/frontend_tests/node_tests/presence.js +++ b/frontend_tests/node_tests/presence.js @@ -5,7 +5,7 @@ var return_false = function () { return false; }; set_global('server_events', {}); set_global('blueslip', {}); -set_global('reload', { +set_global('reload_state', { is_in_progress: return_false, }); diff --git a/frontend_tests/node_tests/server_events.js b/frontend_tests/node_tests/server_events.js index 1d1310b9f4..a55a0cec3b 100644 --- a/frontend_tests/node_tests/server_events.js +++ b/frontend_tests/node_tests/server_events.js @@ -17,7 +17,7 @@ set_global('home_msg_list', { selected_id: function () {return 1;}, }); set_global('page_params', {test_suite: false}); -set_global('reload', { +set_global('reload_state', { is_in_progress: function () {return false;}, }); diff --git a/frontend_tests/node_tests/transmit.js b/frontend_tests/node_tests/transmit.js index 6f0c7da0fc..6626e10d71 100644 --- a/frontend_tests/node_tests/transmit.js +++ b/frontend_tests/node_tests/transmit.js @@ -8,6 +8,7 @@ set_global('page_params', { set_global('channel', {}); set_global('navigator', {}); set_global('reload', {}); +set_global('reload_state', {}); set_global('socket', {}); set_global('Socket', function () { return global.socket; @@ -136,7 +137,7 @@ run_test('transmit_message_ajax', () => { run_test('transmit_message_ajax_reload_pending', () => { var success = function () { throw 'unexpected success'; }; - reload.is_pending = function () { + reload_state.is_pending = function () { return true; }; diff --git a/static/js/activity.js b/static/js/activity.js index 5529774b5c..bdbfd291fa 100644 --- a/static/js/activity.js +++ b/static/js/activity.js @@ -295,7 +295,7 @@ exports.update_huddles = function () { }; function focus_ping(want_redraw) { - if (reload.is_in_progress()) { + if (reload_state.is_in_progress()) { blueslip.log("Skipping querying presence because reload in progress"); return; } diff --git a/static/js/bundles/app.js b/static/js/bundles/app.js index 74cbe8482f..4a58949716 100644 --- a/static/js/bundles/app.js +++ b/static/js/bundles/app.js @@ -78,6 +78,7 @@ import "js/message_list.js"; import "js/message_live_update.js"; import "js/narrow_state.js"; import "js/narrow.js"; +import "js/reload_state.js"; import "js/reload.js"; import "js/compose_fade.js"; import "js/fenced_code.js"; diff --git a/static/js/compose_actions.js b/static/js/compose_actions.js index 0dab5b9558..65c9589db5 100644 --- a/static/js/compose_actions.js +++ b/static/js/compose_actions.js @@ -192,7 +192,7 @@ function same_recipient_as_before(msg_type, opts) { exports.start = function (msg_type, opts) { exports.autosize_message_content(); - if (reload.is_in_progress()) { + if (reload_state.is_in_progress()) { return; } notifications.clear_compose_notifications(); diff --git a/static/js/people.js b/static/js/people.js index 6ab66abe9b..8ae4ecebf4 100644 --- a/static/js/people.js +++ b/static/js/people.js @@ -783,7 +783,7 @@ exports.report_late_add = function (user_id, email) { // types of realms. var msg = 'Added user late: user_id=' + user_id + ' email=' + email; - if (reload.is_in_progress()) { + if (reload_state.is_in_progress()) { blueslip.log(msg); } else { blueslip.error(msg); diff --git a/static/js/presence.js b/static/js/presence.js index d8f18684ed..45ed2313b5 100644 --- a/static/js/presence.js +++ b/static/js/presence.js @@ -104,7 +104,7 @@ exports.set_info = function (presences, server_timestamp) { if (!people.is_current_user(this_email)) { var person = people.get_by_email(this_email); if (person === undefined) { - if (!(server_events.suspect_offline || reload.is_in_progress())) { + if (!(server_events.suspect_offline || reload_state.is_in_progress())) { // If we're online, and we get a user who we don't // know about in the presence data, throw an error. blueslip.error('Unknown email in presence data: ' + this_email); diff --git a/static/js/reload.js b/static/js/reload.js index d8c321edc9..d14661d8ff 100644 --- a/static/js/reload.js +++ b/static/js/reload.js @@ -3,17 +3,6 @@ var reload = (function () { var exports = {}; -var reload_in_progress = false; -var reload_pending = false; - -exports.is_pending = function () { - return reload_pending; -}; - -exports.is_in_progress = function () { - return reload_in_progress; -}; - function preserve_state(send_after_reload, save_pointer, save_narrow, save_compose) { if (!localstorage.supported()) { // If local storage is not supported by the browser, we can't @@ -181,7 +170,7 @@ exports.initialize = function () { }; function do_reload_app(send_after_reload, save_pointer, save_narrow, save_compose, message) { - if (reload_in_progress) { + if (reload_state.is_in_progress()) { blueslip.log("do_reload_app: Doing nothing since reload_in_progress"); return; } @@ -203,7 +192,7 @@ function do_reload_app(send_after_reload, save_pointer, save_narrow, save_compos // TODO: We need a better API for showing messages. ui_report.message(message, $("#reloading-application")); blueslip.log('Starting server requested page reload'); - reload_in_progress = true; + reload_state.set_state_to_in_progress(); // Sometimes the window.location.reload that we attempt has no // immediate effect (likely by browsers trying to save power by @@ -253,10 +242,10 @@ exports.initiate = function (options) { options.message); } - if (reload_pending) { + if (reload_state.is_pending()) { return; } - reload_pending = true; + reload_state.set_state_to_pending(); // If the user is composing a message, reload if they become idle // while composing. If they finish or cancel the compose, wait @@ -315,7 +304,7 @@ window.addEventListener('beforeunload', function () { // happens before the navigation is complete the user is kept captive at // zulip. blueslip.log("Setting reload_in_progress in beforeunload handler"); - reload_in_progress = true; + reload_state.set_state_to_in_progress(); }); diff --git a/static/js/reload_state.js b/static/js/reload_state.js new file mode 100644 index 0000000000..87a02d3b1b --- /dev/null +++ b/static/js/reload_state.js @@ -0,0 +1,43 @@ +var reload_state = (function () { + +var exports = {}; + +/* + We want his module to load pretty early in the process + of starting the app, so that people.js can load early. + All the heavy lifting for reload logic happens in + reload.js, which has lots of UI dependencies. If we + didn't split out this module, our whole dependency tree + would be kind of upside down. +*/ + +var reload_in_progress = false; +var reload_pending = false; + +exports.is_pending = function () { + return reload_pending; +}; + +exports.is_in_progress = function () { + return reload_in_progress; +}; + +exports.set_state_to_pending = function () { + // Why do we never set this back to false? + // Because the reload is gonna happen next. :) + // I was briefly confused by this, hence the comment. + reload_pending = true; +}; + +exports.set_state_to_in_progress = function () { + reload_in_progress = true; +}; + + +return exports; +}()); + +if (typeof module !== 'undefined') { + module.exports = reload_state; +} +window.reload_state = reload_state; diff --git a/static/js/server_events.js b/static/js/server_events.js index f8e66aebb4..684d6adf2b 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -131,7 +131,7 @@ function get_events_success(events) { function get_events(options) { options = _.extend({dont_block: false}, options); - if (reload.is_in_progress()) { + if (reload_state.is_in_progress()) { return; } diff --git a/static/js/transmit.js b/static/js/transmit.js index 6f634b327c..7690408741 100644 --- a/static/js/transmit.js +++ b/static/js/transmit.js @@ -30,7 +30,7 @@ function send_message_ajax(request, success, error) { data: request, success: success, error: function (xhr, error_type) { - if (error_type !== 'timeout' && reload.is_pending()) { + if (error_type !== 'timeout' && reload_state.is_pending()) { // The error might be due to the server changing reload.initiate({immediate: true, save_pointer: true,