diff --git a/lib/gui/app/components/drive-selector/controllers/drive-selector.js b/lib/gui/app/components/drive-selector/controllers/drive-selector.js index f61acc5d..64d7f348 100644 --- a/lib/gui/app/components/drive-selector/controllers/drive-selector.js +++ b/lib/gui/app/components/drive-selector/controllers/drive-selector.js @@ -23,6 +23,7 @@ const constraints = require('../../../../../shared/drive-constraints') const analytics = require('../../../modules/analytics') const availableDrives = require('../../../../../shared/models/available-drives') const selectionState = require('../../../../../shared/models/selection-state') +const utils = require('../../../../../shared/utils') module.exports = function ( $q, @@ -173,66 +174,17 @@ module.exports = function ( } /** - * @summary Memoize ImmutableJS list reference + * @summary Memoized getDrives function * @function - * @private + * @public * - * @description - * This workaround is needed to avoid AngularJS from getting - * caught in an infinite digest loop when using `ngRepeat` - * over a function that returns a mutable version of an - * ImmutableJS object. - * - * The problem is that every time you call `myImmutableObject.toJS()` - * you will get a new object, whose reference is different from - * the one you previously got, even if the data is exactly the same. - * - * @param {Function} func - function that returns an ImmutableJS list - * @returns {Function} memoized function + * @returns {Array} - memoized list of drives * * @example - * const getList = () => { - * return Store.getState().toJS().myList; - * }; - * - * const memoizedFunction = memoizeImmutableListReference(getList); + * const drives = DriveSelectorController.getDrives() + * // Do something with drives */ - this.memoizeImmutableListReference = (func) => { - let previousTuples = [] - - return (...restArgs) => { - let areArgsInTuple = false - let state = Reflect.apply(func, this, restArgs) - - previousTuples = _.map(previousTuples, ([ oldArgs, oldState ]) => { - if (angular.equals(oldArgs, restArgs)) { - areArgsInTuple = true - - if (angular.equals(state, oldState)) { - // Use the previously memoized state for this argument - state = oldState - } - - // Update the tuple state - return [ oldArgs, state ] - } - - // Return the tuple unchanged - return [ oldArgs, oldState ] - }) - - // Add the state associated with these args to be memoized - if (!areArgsInTuple) { - previousTuples.push([ restArgs, state ]) - } - - return state - } - } - - this.getDrives = this.memoizeImmutableListReference(() => { - return this.drives.getDrives() - }) + this.getDrives = utils.memoize(this.drives.getDrives, angular.equals) /** * @summary Get a drive's compatibility status object(s) @@ -253,9 +205,9 @@ module.exports = function ( * // do something * } */ - this.getDriveStatuses = this.memoizeImmutableListReference((drive) => { + this.getDriveStatuses = utils.memoize((drive) => { return this.constraints.getDriveImageCompatibilityStatuses(drive, this.state.getImage()) - }) + }, angular.equals) /** * @summary Keyboard event drive toggling diff --git a/lib/shared/utils.js b/lib/shared/utils.js index 45fab2ee..8a840100 100644 --- a/lib/shared/utils.js +++ b/lib/shared/utils.js @@ -78,3 +78,62 @@ exports.percentageToFloat = (percentage) => { return percentage / exports.PERCENTAGE_MAXIMUM } + +/** + * @summary Memoize a function + * @function + * @private + * + * @description + * This workaround is needed to avoid AngularJS from getting + * caught in an infinite digest loop when using `ngRepeat` + * over a function that returns a mutable version of an + * ImmutableJS object. + * + * The problem is that every time you call `myImmutableObject.toJS()` + * you will get a new object, whose reference is different from + * the one you previously got, even if the data is exactly the same. + * + * @param {Function} func - function that returns an ImmutableJS list + * @param {Function} comparer - function to compare old and new args and state + * @returns {Function} memoized function + * + * @example + * const getList = () => { + * return Store.getState().toJS().myList; + * }; + * + * const memoizedFunction = memoize(getList, angular.equals); + */ +exports.memoize = (func, comparer) => { + let previousTuples = [] + + return (...restArgs) => { + let areArgsInTuple = false + let state = Reflect.apply(func, this, restArgs) + + previousTuples = _.map(previousTuples, ([ oldArgs, oldState ]) => { + if (comparer(oldArgs, restArgs)) { + areArgsInTuple = true + + if (comparer(state, oldState)) { + // Use the previously memoized state for this argument + state = oldState + } + + // Update the tuple state + return [ oldArgs, state ] + } + + // Return the tuple unchanged + return [ oldArgs, oldState ] + }) + + // Add the state associated with these args to be memoized + if (!areArgsInTuple) { + previousTuples.push([ restArgs, state ]) + } + + return state + } +} diff --git a/tests/gui/components/drive-selector.spec.js b/tests/gui/components/drive-selector.spec.js index f0e8fe0b..b8852d18 100644 --- a/tests/gui/components/drive-selector.spec.js +++ b/tests/gui/components/drive-selector.spec.js @@ -20,6 +20,7 @@ const _ = require('lodash') const m = require('mochainon') const angular = require('angular') require('angular-mocks') +const utils = require('../../../lib/shared/utils') describe('Browser: DriveSelector', function () { beforeEach(angular.mock.module( @@ -27,64 +28,9 @@ describe('Browser: DriveSelector', function () { )) describe('DriveSelectorController', function () { - let $controller - let $rootScope - let $q - let $uibModalInstance - let WarningModalService - - let controller - - beforeEach(angular.mock.inject(function ( - _$controller_, - _$rootScope_, - _$q_, - _WarningModalService_ - ) { - $controller = _$controller_ - $rootScope = _$rootScope_ - $q = _$q_ - $uibModalInstance = {} - WarningModalService = _WarningModalService_ - })) - - beforeEach(() => { - controller = $controller('DriveSelectorController', { - $scope: $rootScope.$new(), - $q, - $uibModalInstance, - WarningModalService - }) - }) - - describe('.memoizeImmutableListReference()', function () { - it('constant true should return memoized true', function () { - const memoizedConstTrue = controller.memoizeImmutableListReference(_.constant(true)) - m.chai.expect(memoizedConstTrue()).to.be.true - }) - - it('should reflect state changes', function () { - let stateA = false - const memoizedStateA = controller.memoizeImmutableListReference(() => { - return stateA - }) - - m.chai.expect(memoizedStateA()).to.be.false - - stateA = true - - m.chai.expect(memoizedStateA()).to.be.true - }) - - it('should reflect different arguments', function () { - const memoizedParameter = controller.memoizeImmutableListReference(_.identity) - - m.chai.expect(memoizedParameter(false)).to.be.false - m.chai.expect(memoizedParameter(true)).to.be.true - }) - + describe('.memoize()', function () { it('should handle equal angular objects with different hashes', function () { - const memoizedParameter = controller.memoizeImmutableListReference(_.identity) + const memoizedParameter = utils.memoize(_.identity, angular.equals) const angularObjectA = { $$hashKey: 1, keyA: true diff --git a/tests/shared/utils.spec.js b/tests/shared/utils.spec.js index 2a1f2d66..88625c99 100644 --- a/tests/shared/utils.spec.js +++ b/tests/shared/utils.spec.js @@ -16,6 +16,7 @@ 'use strict' +const _ = require('lodash') const m = require('mochainon') const utils = require('../../lib/shared/utils') @@ -125,4 +126,31 @@ describe('Shared: Utils', function () { }).to.throw('Invalid percentage: 100.01') }) }) + + describe('.memoize()', function () { + it('constant true should return memoized true', function () { + const memoizedConstTrue = utils.memoize(_.constant(true), _.isEqual) + m.chai.expect(memoizedConstTrue()).to.be.true + }) + + it('should reflect state changes', function () { + let stateA = false + const memoizedStateA = utils.memoize(() => { + return stateA + }, _.isEqual) + + m.chai.expect(memoizedStateA()).to.be.false + + stateA = true + + m.chai.expect(memoizedStateA()).to.be.true + }) + + it('should reflect different arguments', function () { + const memoizedParameter = utils.memoize(_.identity, _.isEqual) + + m.chai.expect(memoizedParameter(false)).to.be.false + m.chai.expect(memoizedParameter(true)).to.be.true + }) + }) })