Skip to content

Commit

Permalink
Chore: Removing autobind from Preview.js (#500)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tonyjin
tonyjin authored Nov 21, 2017
1 parent 6d534be commit 6d61b9d
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 55 deletions.
38 changes: 21 additions & 17 deletions src/lib/Preview.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable import/first */
import './polyfill';
import autobind from 'autobind-decorator';
import EventEmitter from 'events';
import throttle from 'lodash.throttle';
import cloneDeep from 'lodash.clonedeep';
Expand Down Expand Up @@ -54,7 +53,6 @@ const LOG_RETRY_COUNT = 3; // number of times to retry logging preview event
// and not when preview is instantiated, which is too late.
const PREVIEW_LOCATION = findScriptLocation(PREVIEW_SCRIPT_NAME, document.currentScript);

@autobind
class Preview extends EventEmitter {
/** @property {boolean} - Whether preview is open */
open = false;
Expand Down Expand Up @@ -138,6 +136,18 @@ class Preview extends EventEmitter {
this.cache = new Cache();
this.ui = new PreviewUI();
this.browserInfo = Browser.getBrowserInfo();

// Bind context for callbacks
this.print = this.print.bind(this);
this.handleTokenResponse = this.handleTokenResponse.bind(this);
this.handleFileInfoResponse = this.handleFileInfoResponse.bind(this);
this.handleFetchError = this.handleFetchError.bind(this);
this.handleViewerEvents = this.handleViewerEvents.bind(this);
this.triggerError = this.triggerError.bind(this);
this.throttledMousemoveHandler = this.getGlobalMousemoveHandler().bind(this);
this.navigateLeft = this.navigateLeft.bind(this);
this.navigateRight = this.navigateRight.bind(this);
this.keydownHandler = this.keydownHandler.bind(this);
}

/**
Expand Down Expand Up @@ -582,8 +592,8 @@ class Preview extends EventEmitter {

// Fetch access tokens before proceeding
getTokens(this.file.id, this.previewOptions.token)
.then(this.loadPreviewWithTokens)
.catch(this.triggerFetchError);
.then(this.handleTokenResponse)
.catch(this.handleFetchError);
}

/**
Expand All @@ -593,7 +603,7 @@ class Preview extends EventEmitter {
* @param {Object} tokenMap - Map of file ID to access token
* @return {void}
*/
loadPreviewWithTokens(tokenMap) {
handleTokenResponse(tokenMap) {
// If this is a retry, short-circuit and load from server
if (this.retryCount > 0) {
this.loadFromServer();
Expand All @@ -609,7 +619,7 @@ class Preview extends EventEmitter {
this.keydownHandler,
this.navigateLeft,
this.navigateRight,
this.getGlobalMousemoveHandler()
this.throttledMousemoveHandler
);

// Setup loading UI and progress bar
Expand Down Expand Up @@ -747,8 +757,8 @@ class Preview extends EventEmitter {

const fileInfoUrl = appendQueryParams(getURL(this.file.id, apiHost), queryParams);
get(fileInfoUrl, this.getRequestHeaders())
.then(this.handleLoadResponse)
.catch(this.triggerFetchError);
.then(this.handleFileInfoResponse)
.catch(this.handleFetchError);
}

/**
Expand All @@ -758,7 +768,7 @@ class Preview extends EventEmitter {
* @param {Object} file - File object
* @return {void}
*/
handleLoadResponse(file) {
handleFileInfoResponse(file) {
// If preview is closed or response comes back for an incorrect file, don't do anything
if (!this.open || (this.file && this.file.id !== file.id)) {
return;
Expand Down Expand Up @@ -1058,7 +1068,7 @@ class Preview extends EventEmitter {
* @param {Object} err Error object
* @return {void}
*/
triggerFetchError(err) {
handleFetchError(err) {
// If preview is closed don't do anything
if (!this.open) {
return;
Expand Down Expand Up @@ -1227,11 +1237,7 @@ class Preview extends EventEmitter {
* @return {Function} Throttled mousemove handler
*/
getGlobalMousemoveHandler() {
if (this.throttledMousemoveHandler) {
return this.throttledMousemoveHandler;
}

this.throttledMousemoveHandler = throttle(
return throttle(
() => {
clearTimeout(this.timeoutHandler);

Expand Down Expand Up @@ -1262,8 +1268,6 @@ class Preview extends EventEmitter {
MOUSEMOVE_THROTTLE - 500,
true
);

return this.throttledMousemoveHandler;
}

/**
Expand Down
69 changes: 31 additions & 38 deletions src/lib/__tests__/Preview-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ describe('lib/Preview', () => {
});

stubs.getTokens = sandbox.stub(tokens, 'default').returns(stubs.promise);
stubs.loadPreviewWithTokens = sandbox.stub(preview, 'loadPreviewWithTokens');
stubs.handleTokenResponse = sandbox.stub(preview, 'handleTokenResponse');
stubs.get = sandbox.stub(preview.cache, 'get');
stubs.destroy = sandbox.stub(preview, 'destroy');
});
Expand Down Expand Up @@ -757,7 +757,7 @@ describe('lib/Preview', () => {
preview.load({ id: '123' });
return stubs.promise.then(() => {
expect(stubs.getTokens).to.be.calledWith('123', 'token');
expect(stubs.loadPreviewWithTokens).to.be.called;
expect(stubs.handleTokenResponse).to.be.called;
});
});

Expand All @@ -767,12 +767,12 @@ describe('lib/Preview', () => {
preview.load('0');
return stubs.promise.then(() => {
expect(stubs.getTokens).to.be.calledWith('0', 'token');
expect(stubs.loadPreviewWithTokens).to.be.called;
expect(stubs.handleTokenResponse).to.be.called;
});
});
});

describe('loadPreviewWithTokens()', () => {
describe('handleTokenResponse()', () => {
beforeEach(() => {
stubs.loadFromServer = sandbox.stub(preview, 'loadFromServer');
stubs.parseOptions = sandbox.stub(preview, 'parseOptions');
Expand All @@ -789,34 +789,34 @@ describe('lib/Preview', () => {
it('should short circuit and load from server if it is a retry', () => {
preview.retryCount = 1;

preview.loadPreviewWithTokens({});
preview.handleTokenResponse({});
expect(stubs.loadFromServer).to.be.called;
expect(stubs.parseOptions).to.not.be.called;
});

it('should parse the preview options', () => {
preview.retryCount = 0;

preview.loadPreviewWithTokens({});
preview.handleTokenResponse({});
expect(stubs.parseOptions).to.be.called;
});

it('should setup the container and show the loading indicator and progress bar', () => {
preview.loadPreviewWithTokens({});
preview.handleTokenResponse({});
expect(stubs.setup).to.be.called;
expect(stubs.showLoadingIndicator).to.be.called;
expect(stubs.startProgressBar).to.be.called;
});

it('should show navigation', () => {
preview.loadPreviewWithTokens({});
preview.handleTokenResponse({});
expect(stubs.showNavigation).to.be.called;
});

it('should load from cache if the file is valid', () => {
stubs.checkFileValid.returns(true);

preview.loadPreviewWithTokens({});
preview.handleTokenResponse({});
expect(stubs.cacheFile).to.be.calledWith(preview.cache, preview.file);
expect(stubs.loadFromCache).to.be.called;
expect(stubs.loadFromServer).to.not.be.called;
Expand All @@ -825,7 +825,7 @@ describe('lib/Preview', () => {
it('should load from the server on a cache miss', () => {
stubs.checkFileValid.returns(false);

preview.loadPreviewWithTokens({});
preview.handleTokenResponse({});
expect(stubs.loadFromCache).to.not.be.called;
expect(stubs.loadFromServer).to.be.called;
});
Expand Down Expand Up @@ -1008,8 +1008,8 @@ describe('lib/Preview', () => {
beforeEach(() => {
stubs.promise = Promise.resolve('file');
stubs.get = sandbox.stub(util, 'get').returns(stubs.promise);
stubs.handleLoadResponse = sandbox.stub(preview, 'handleLoadResponse');
stubs.triggerFetchError = sandbox.stub(preview, 'triggerFetchError');
stubs.handleFileInfoResponse = sandbox.stub(preview, 'handleFileInfoResponse');
stubs.handleFetchError = sandbox.stub(preview, 'handleFetchError');
stubs.getURL = sandbox.stub(file, 'getURL').returns('/get_url');
preview.file = {
id: 0
Expand All @@ -1021,13 +1021,13 @@ describe('lib/Preview', () => {
expect(stubs.get).to.be.called;
expect(stubs.getURL).to.be.called;
return stubs.promise.then(() => {
expect(stubs.handleLoadResponse).to.be.called;
expect(stubs.triggerFetchError).to.not.be.called;
expect(stubs.handleFileInfoResponse).to.be.called;
expect(stubs.handleFetchError).to.not.be.called;
});
});
});

describe('handleLoadResponse()', () => {
describe('handleFileInfoResponse()', () => {
beforeEach(() => {
preview.logger = {
setFile: sandbox.stub(),
Expand Down Expand Up @@ -1056,7 +1056,7 @@ describe('lib/Preview', () => {

it('should do nothing if the preview is closed', () => {
preview.open = false;
preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(stubs.set).to.not.be.called;
});

Expand All @@ -1067,7 +1067,7 @@ describe('lib/Preview', () => {
};
stubs.file.id = 1;

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(stubs.set).to.not.be.called;
});

Expand All @@ -1077,7 +1077,7 @@ describe('lib/Preview', () => {
id: 0
};

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(preview.file).to.equal(stubs.file);
expect(preview.logger.setFile).to.be.called;
});
Expand All @@ -1096,7 +1096,7 @@ describe('lib/Preview', () => {

stubs.file.file_version.sha1 = 0;

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(stubs.get).to.be.calledWith(stubs.file.id);
expect(stubs.set).to.be.calledWith(stubs.file.id);
expect(stubs.loadViewer).to.not.be.called;
Expand All @@ -1117,7 +1117,7 @@ describe('lib/Preview', () => {

stubs.file.file_version.sha1 = 0;

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(stubs.set).to.be.not.be.called;
});

Expand All @@ -1129,7 +1129,7 @@ describe('lib/Preview', () => {

stubs.get.returns(false);

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(preview.logger.setCacheStale).to.be.called;
expect(stubs.loadViewer).to.be.called;
});
Expand All @@ -1142,7 +1142,7 @@ describe('lib/Preview', () => {

stubs.checkFileValid.returns(false);

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(preview.logger.setCacheStale).to.be.called;
expect(stubs.loadViewer).to.be.called;
});
Expand All @@ -1161,7 +1161,7 @@ describe('lib/Preview', () => {

stubs.file.file_version.sha1 = 2;

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(preview.logger.setCacheStale).to.be.called;
expect(stubs.loadViewer).to.be.called;
});
Expand All @@ -1181,7 +1181,7 @@ describe('lib/Preview', () => {

stubs.file.file_version.sha1 = 2;

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(preview.logger.setCacheStale).to.be.called;
expect(stubs.loadViewer).to.be.called;
});
Expand All @@ -1194,7 +1194,7 @@ describe('lib/Preview', () => {

stubs.get.throws(new Error());

preview.handleLoadResponse(stubs.file);
preview.handleFileInfoResponse(stubs.file);
expect(stubs.triggerError).to.be.called;
});
});
Expand Down Expand Up @@ -1641,7 +1641,7 @@ describe('lib/Preview', () => {
});
});

describe('triggerFetchError()', () => {
describe('handleFetchError()', () => {
beforeEach(() => {
stubs.unset = sandbox.stub(preview.cache, 'unset');
stubs.triggerError = sandbox.stub(preview, 'triggerError');
Expand All @@ -1659,7 +1659,7 @@ describe('lib/Preview', () => {
};
preview.open = false;

preview.triggerFetchError(stubs.error);
preview.handleFetchError(stubs.error);
expect(stubs.unset).to.not.be.called;
});

Expand All @@ -1669,7 +1669,7 @@ describe('lib/Preview', () => {
};
preview.open = true;

preview.triggerFetchError(stubs.error);
preview.handleFetchError(stubs.error);
expect(stubs.unset).to.be.called;
});

Expand All @@ -1680,7 +1680,7 @@ describe('lib/Preview', () => {
preview.open = true;
preview.retryCount = 6;

preview.triggerFetchError(stubs.error);
preview.handleFetchError(stubs.error);
expect(stubs.triggerError).to.be.called;
});

Expand All @@ -1692,7 +1692,7 @@ describe('lib/Preview', () => {
preview.retryCount = 6;
stubs.error.response.status = 429;

preview.triggerFetchError(stubs.error);
preview.handleFetchError(stubs.error);
try {
expect(stubs.triggerError).to.be.calledWith(new Error(__('error_rate_limit')));
} catch (e) {
Expand All @@ -1709,7 +1709,7 @@ describe('lib/Preview', () => {
preview.retryCount = 1;
preview.file.id = 1;

preview.triggerFetchError(stubs.error);
preview.handleFetchError(stubs.error);
expect(stubs.triggerError).to.not.be.called;

clock.tick(RETRY_TIMEOUT + 1);
Expand Down Expand Up @@ -1907,13 +1907,6 @@ describe('lib/Preview', () => {
});

describe('getGlobalMousemoveHandler()', () => {
it('should return the throttled mouse move handler if it already exists', () => {
preview.throttledMousemoveHandler = true;

const handler = preview.getGlobalMousemoveHandler();
expect(handler).to.be.true;
});

it('should clear the timeout handler and do nothing if the container doesn\'t exist', () => {
preview.container = false;

Expand Down

0 comments on commit 6d61b9d

Please sign in to comment.