From 7f8e8177d0f98f5e3de4248738328f8f13ad245f Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Tue, 29 Aug 2017 18:30:21 -0500 Subject: [PATCH] [RN] Refactor "Keep track of ongoing network requests" and "Show a progress indicator in the BlankPage" I'm not saying that the two commits in question were wrong or worse than what I'm offering. Anyway, I think what I'm offering brings: * Compliance with expectations i.e. the middleware doesn't compute the next state from the current state, the reducer does; * Clarity and/or simplicity i.e. there's no global variable (reqIndex), there's no need for the term "index" (a.k.a "reqIndex") in the redux store. * By renaming net-interceptor to network-activity feels like it's preparing the feature to implement a NetworkActivityIndicator React Component which will take on more of the knowledge about the specifics of what is the network activity redux state exactly, is it maintained by interception or some other mechanism, and abstracts it in the feature itself allowing outsiders to merely render a React Component. --- react/features/app/components/App.native.js | 2 +- .../mobile/net-interceptor/actionTypes.js | 11 --- .../mobile/net-interceptor/functions.js | 75 ----------------- .../mobile/net-interceptor/middleware.js | 25 ------ .../mobile/net-interceptor/reducer.js | 15 ---- .../mobile/network-activity/actionTypes.js | 37 +++++++++ .../index.js | 0 .../mobile/network-activity/middleware.js | 81 +++++++++++++++++++ .../mobile/network-activity/reducer.js | 60 ++++++++++++++ .../welcome/components/BlankPage.native.js | 7 +- 10 files changed, 184 insertions(+), 129 deletions(-) delete mode 100644 react/features/mobile/net-interceptor/actionTypes.js delete mode 100644 react/features/mobile/net-interceptor/functions.js delete mode 100644 react/features/mobile/net-interceptor/middleware.js delete mode 100644 react/features/mobile/net-interceptor/reducer.js create mode 100644 react/features/mobile/network-activity/actionTypes.js rename react/features/mobile/{net-interceptor => network-activity}/index.js (100%) create mode 100644 react/features/mobile/network-activity/middleware.js create mode 100644 react/features/mobile/network-activity/reducer.js diff --git a/react/features/app/components/App.native.js b/react/features/app/components/App.native.js index f58bc9b73..647d37a0d 100644 --- a/react/features/app/components/App.native.js +++ b/react/features/app/components/App.native.js @@ -8,7 +8,7 @@ import '../../mobile/audio-mode'; import '../../mobile/background'; import '../../mobile/external-api'; import '../../mobile/full-screen'; -import '../../mobile/net-interceptor'; +import '../../mobile/network-activity'; import '../../mobile/permissions'; import '../../mobile/proximity'; import '../../mobile/wake-lock'; diff --git a/react/features/mobile/net-interceptor/actionTypes.js b/react/features/mobile/net-interceptor/actionTypes.js deleted file mode 100644 index 1a91cd5d2..000000000 --- a/react/features/mobile/net-interceptor/actionTypes.js +++ /dev/null @@ -1,11 +0,0 @@ -/** - * The type of redux action to update the pending network requests mapping. - * - * { - * type: UPDATE_NETWORK_REQUESTS, - * requests: Object - * } - * - * @protected - */ -export const UPDATE_NETWORK_REQUESTS = Symbol('UPDATE_NETWORK_REQUESTS'); diff --git a/react/features/mobile/net-interceptor/functions.js b/react/features/mobile/net-interceptor/functions.js deleted file mode 100644 index 931ac73ee..000000000 --- a/react/features/mobile/net-interceptor/functions.js +++ /dev/null @@ -1,75 +0,0 @@ -import _ from 'lodash'; -import XHRInterceptor from 'react-native/Libraries/Network/XHRInterceptor'; - -import { UPDATE_NETWORK_REQUESTS } from './actionTypes'; - -/** - * Global index for keeping track of XHR requests. - * @type {number} - */ -let reqIndex = 0; - -/** - * Starts intercepting network requests. Only XHR requests are supported at the - * moment. - * - * Ongoing request information is kept in redux, and it's removed as soon as a - * given request is complete. - * - * @param {Object} store - The redux store. - * @returns {void} - */ -export function startNetInterception({ dispatch, getState }) { - XHRInterceptor.setOpenCallback((method, url, xhr) => { - xhr._reqIndex = reqIndex++; - - const requests = getState()['features/net-interceptor'].requests || {}; - const newRequests = _.cloneDeep(requests); - const request = { - method, - url - }; - - newRequests[xhr._reqIndex] = request; - - dispatch({ - type: UPDATE_NETWORK_REQUESTS, - requests: newRequests - }); - }); - - XHRInterceptor.setResponseCallback((...args) => { - const xhr = args.slice(-1)[0]; - - if (typeof xhr._reqIndex === 'undefined') { - return; - } - - const requests = getState()['features/net-interceptor'].requests || {}; - const newRequests = _.cloneDeep(requests); - - delete newRequests[xhr._reqIndex]; - - dispatch({ - type: UPDATE_NETWORK_REQUESTS, - requests: newRequests - }); - }); - - XHRInterceptor.enableInterception(); -} - -/** - * Stops intercepting network requests. - * - * @param {Object} store - The redux store. - * @returns {void} - */ -export function stopNetInterception({ dispatch }) { - XHRInterceptor.disableInterception(); - - dispatch({ - type: UPDATE_NETWORK_REQUESTS, - requests: {} - }); -} diff --git a/react/features/mobile/net-interceptor/middleware.js b/react/features/mobile/net-interceptor/middleware.js deleted file mode 100644 index 87e80c653..000000000 --- a/react/features/mobile/net-interceptor/middleware.js +++ /dev/null @@ -1,25 +0,0 @@ -import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../../app'; -import { MiddlewareRegistry } from '../../base/redux'; - -import { startNetInterception, stopNetInterception } from './functions'; - -/** - * Middleware which captures app startup and conference actions in order to - * clear the image cache. - * - * @returns {Function} - */ -MiddlewareRegistry.register(store => next => action => { - const result = next(action); - - switch (action.type) { - case APP_WILL_MOUNT: - startNetInterception(store); - break; - case APP_WILL_UNMOUNT: - stopNetInterception(store); - break; - } - - return result; -}); diff --git a/react/features/mobile/net-interceptor/reducer.js b/react/features/mobile/net-interceptor/reducer.js deleted file mode 100644 index 0f9a4df22..000000000 --- a/react/features/mobile/net-interceptor/reducer.js +++ /dev/null @@ -1,15 +0,0 @@ -import { ReducerRegistry } from '../../base/redux'; - -import { UPDATE_NETWORK_REQUESTS } from './actionTypes'; - -ReducerRegistry.register('features/net-interceptor', (state = {}, action) => { - switch (action.type) { - case UPDATE_NETWORK_REQUESTS: - return { - ...state, - requests: action.requests - }; - } - - return state; -}); diff --git a/react/features/mobile/network-activity/actionTypes.js b/react/features/mobile/network-activity/actionTypes.js new file mode 100644 index 000000000..f44530202 --- /dev/null +++ b/react/features/mobile/network-activity/actionTypes.js @@ -0,0 +1,37 @@ +/** + * The type of redux action to add a network request to the redux store/state. + * + * { + * type: _ADD_NETWORK_REQUEST, + * request: Object + * } + * + * @protected + */ +export const _ADD_NETWORK_REQUEST = Symbol('_ADD_NETWORK_REQUEST'); + +/** + * The type of redux action to remove all network requests from the redux + * store/state. + * + * { + * type: _REMOVE_ALL_NETWORK_REQUESTS, + * } + * + * @protected + */ +export const _REMOVE_ALL_NETWORK_REQUESTS + = Symbol('_REMOVE_ALL_NETWORK_REQUESTS'); + +/** + * The type of redux action to remove a network request from the redux + * store/state. + * + * { + * type: _REMOVE_NETWORK_REQUEST, + * request: Object + * } + * + * @protected + */ +export const _REMOVE_NETWORK_REQUEST = Symbol('_REMOVE_NETWORK_REQUEST'); diff --git a/react/features/mobile/net-interceptor/index.js b/react/features/mobile/network-activity/index.js similarity index 100% rename from react/features/mobile/net-interceptor/index.js rename to react/features/mobile/network-activity/index.js diff --git a/react/features/mobile/network-activity/middleware.js b/react/features/mobile/network-activity/middleware.js new file mode 100644 index 000000000..09d686d24 --- /dev/null +++ b/react/features/mobile/network-activity/middleware.js @@ -0,0 +1,81 @@ +/* @flow */ + +import XHRInterceptor from 'react-native/Libraries/Network/XHRInterceptor'; + +import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../../app'; +import { MiddlewareRegistry } from '../../base/redux'; + +import { + _ADD_NETWORK_REQUEST, + _REMOVE_ALL_NETWORK_REQUESTS, + _REMOVE_NETWORK_REQUEST +} from './actionTypes'; + +/** + * Middleware which captures app startup and conference actions in order to + * clear the image cache. + * + * @returns {Function} + */ +MiddlewareRegistry.register(store => next => action => { + const result = next(action); + + switch (action.type) { + case APP_WILL_MOUNT: + _startNetInterception(store); + break; + + case APP_WILL_UNMOUNT: + _stopNetInterception(store); + break; + } + + return result; +}); + +/** + * Starts intercepting network requests. Only XHR requests are supported at the + * moment. + * + * Ongoing request information is kept in redux, and it's removed as soon as a + * given request is complete. + * + * @param {Object} store - The redux store. + * @private + * @returns {void} + */ +function _startNetInterception({ dispatch }) { + XHRInterceptor.setOpenCallback((method, url, xhr) => dispatch({ + type: _ADD_NETWORK_REQUEST, + request: xhr, + + // The following are not really necessary read anywhere at the time of + // this writing but are provided anyway if the reducer chooses to + // remember them: + method, + url + })); + XHRInterceptor.setResponseCallback((...args) => dispatch({ + type: _REMOVE_NETWORK_REQUEST, + + // XXX The XHR is the last argument of the responseCallback. + request: args[args.length - 1] + })); + + XHRInterceptor.enableInterception(); +} + +/** + * Stops intercepting network requests. + * + * @param {Object} store - The redux store. + * @private + * @returns {void} + */ +function _stopNetInterception({ dispatch }) { + XHRInterceptor.disableInterception(); + + dispatch({ + type: _REMOVE_ALL_NETWORK_REQUESTS + }); +} diff --git a/react/features/mobile/network-activity/reducer.js b/react/features/mobile/network-activity/reducer.js new file mode 100644 index 000000000..13fba2b7f --- /dev/null +++ b/react/features/mobile/network-activity/reducer.js @@ -0,0 +1,60 @@ +/* @flow */ + +import { ReducerRegistry, set } from '../../base/redux'; + +import { + _ADD_NETWORK_REQUEST, + _REMOVE_ALL_NETWORK_REQUESTS, + _REMOVE_NETWORK_REQUEST +} from './actionTypes'; + +/** + * The initial redux state of the feature network-activity. + * + * @type {{ + * requests: Map + * }} + */ +const _INITIAL_STATE = { + /** + * The ongoing network requests i.e. the network request which have been + * added to the redux store/state and have not been removed. + * + * @type {Map} + */ + requests: new Map() +}; + +ReducerRegistry.register( + 'features/network-activity', + (state = _INITIAL_STATE, action) => { + switch (action.type) { + case _ADD_NETWORK_REQUEST: { + const { + type, // eslint-disable-line no-unused-vars + + request: key, + ...value + } = action; + const requests = new Map(state.requests); + + requests.set(key, value); + + return set(state, 'requests', requests); + } + + case _REMOVE_ALL_NETWORK_REQUESTS: + return set(state, 'requests', _INITIAL_STATE.requests); + + case _REMOVE_NETWORK_REQUEST: { + const { request: key } = action; + const requests = new Map(state.requests); + + requests.delete(key); + + return set(state, 'requests', requests); + } + } + + return state; + }); diff --git a/react/features/welcome/components/BlankPage.native.js b/react/features/welcome/components/BlankPage.native.js index 41c5e4e52..e8a64004d 100644 --- a/react/features/welcome/components/BlankPage.native.js +++ b/react/features/welcome/components/BlankPage.native.js @@ -1,3 +1,5 @@ +/* @flow */ + import PropTypes from 'prop-types'; import React from 'react'; import { ActivityIndicator, View } from 'react-native'; @@ -61,10 +63,11 @@ class BlankPage extends AbstractBlankPage { * }} */ function _mapStateToProps(state) { - const { requests } = state['features/net-interceptor']; + const { requests } = state['features/network-activity']; return { - _networkActivity: Boolean(requests && Object.keys(requests).length) + _networkActivity: + Boolean(requests && (requests.length || requests.size)) }; }