Skip to content

Commit

Permalink
Fix: Temporarily disable requireJS if specified to load 3rd party deps (
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press authored Jan 12, 2018
1 parent b59b453 commit 3f26c81
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 14 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ preview.show(fileId, accessToken, {
| showAnnotations | false | Whether annotations and annotation controls are shown. This option will be overridden by viewer-specific annotation options if they are set. See [Box Annotations](https://github.com/box/box-annotations) for more details. |
| showDownload | false | Whether download button is shown |
| useHotkeys | true | Whether hotkeys (keyboard shortcuts) are enabled |
| pauseRequireJS | false | Temporarily disables requireJS to allow Preview's third party dependencies to load |

Access Token
------------
Expand Down
4 changes: 4 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,10 @@ class Preview extends EventEmitter {
// Optional additional query params to append to requests
this.options.queryParams = options.queryParams || {};

// Option to pause requireJS while Preview loads third party dependencies
// RequireJS will be re-enabled on the 'assetsloaded' event fired by Preview
this.options.pauseRequireJS = !!options.pauseRequireJS;

// Prefix any user created loaders before our default ones
this.loaders = (options.loaders || []).concat(loaderList);

Expand Down
6 changes: 6 additions & 0 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,7 @@ describe('lib/Preview', () => {
logoUrl: stubs.logoUrl,
showDownload: true,
showAnnotations: true,
pauseRequireJS: true,
collection: stubs.collection,
loaders: stubs.loaders
};
Expand Down Expand Up @@ -1002,6 +1003,11 @@ describe('lib/Preview', () => {
expect(preview.options.skipServerUpdate).to.be.true;
});

it('should set whether to pause requireJS when loading dependencies', () => {
preview.parseOptions(preview.previewOptions, stubs.tokens);
expect(preview.options.pauseRequireJS).to.be.true;
});

it('should add user created loaders before standard loaders', () => {
const expectedLoaders = stubs.loaders.concat(loaders);

Expand Down
16 changes: 16 additions & 0 deletions src/lib/__tests__/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,22 @@ describe('lib/util', () => {
assert.ok(head.querySelector('script[src="foo"]') instanceof HTMLScriptElement);
assert.ok(head.querySelector('script[src="bar"]') instanceof HTMLScriptElement);
});

it('should clear requireJS until scripts are loaded or fail to load', () => {
window.define = true;
window.require = true;
window.requrejs = true;

return util.loadScripts(['foo', 'bar'], true).catch(() => {
expect(window.define).to.equal(true);
expect(window.require).to.equal(true);
expect(window.requirejs).to.equal(true);
});

expect(window.define).to.equal(undefined);
expect(window.require).to.equal(undefined);
expect(window.requirejs).to.equal(undefined);
});
});

describe('findScriptLocation()', () => {
Expand Down
26 changes: 24 additions & 2 deletions src/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,11 +467,19 @@ export function loadStylesheets(urls) {
*
* @public
* @param {Array} urls - Asset urls
* @param {string} [disableRequireJS] - Should requireJS be temporarily disabled
* @return {Promise} Promise to load scripts
*/
export function loadScripts(urls) {
export function loadScripts(urls, disableRequireJS = false) {
const { head } = document;
const promises = [];
const { define, require, requirejs } = window;

if (disableRequireJS) {
window.define = undefined;
window.require = undefined;
window.requirejs = undefined;
}

urls.forEach((url) => {
if (!head.querySelector(`script[src="${url}"]`)) {
Expand All @@ -486,7 +494,21 @@ export function loadScripts(urls) {
}
});

return Promise.all(promises);
return Promise.all(promises)
.then(() => {
if (disableRequireJS) {
window.define = define;
window.require = require;
window.requirejs = requirejs;
}
})
.catch(() => {
if (disableRequireJS) {
window.define = define;
window.require = require;
window.requirejs = requirejs;
}
});
}

/**
Expand Down
12 changes: 9 additions & 3 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,9 +592,11 @@ class BaseViewer extends EventEmitter {
* @protected
* @param {Array} [js] - js assets
* @param {Array} [css] - css assets
* @param {boolean} [isViewerAsset] is the asset to load third party
* @return {Promise} Promise to load scripts
*/
loadAssets(js, css) {
loadAssets(js, css, isViewerAsset = true) {
const disableRequireJS = isViewerAsset && !!this.options.pauseRequireJS;
// Create an asset path creator function
const { location } = this.options;
const assetUrlCreator = createAssetUrlCreator(location);
Expand All @@ -603,7 +605,11 @@ class BaseViewer extends EventEmitter {
loadStylesheets((css || []).map(assetUrlCreator));

// Then load the scripts needed for this preview
return loadScripts((js || []).map(assetUrlCreator));
return loadScripts((js || []).map(assetUrlCreator), disableRequireJS).then(() => {
if (isViewerAsset) {
this.emit('assetsloaded');
}
});
}

/**
Expand Down Expand Up @@ -697,7 +703,7 @@ class BaseViewer extends EventEmitter {
this.annotationsLoadPromise =
window.BoxAnnotations && this.options.boxAnnotations instanceof window.BoxAnnotations
? Promise.resolve()
: this.loadAssets([ANNOTATIONS_JS], [ANNOTATIONS_CSS]);
: this.loadAssets([ANNOTATIONS_JS], [ANNOTATIONS_CSS], false);
}

/**
Expand Down
32 changes: 24 additions & 8 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -680,19 +680,35 @@ describe('lib/viewers/BaseViewer', () => {
});

describe('loadAssets()', () => {
it('should create an asset URL and load the relevant stylesheets and scripts', () => {
base.options.location = {};
const promise = Promise.resolve();

beforeEach(() => {
sandbox.stub(util, 'createAssetUrlCreator').returns(() => {});
sandbox.stub(util, 'loadStylesheets');
sandbox.stub(util, 'loadScripts').returns(promise);
sandbox.stub(util, 'loadScripts').returns(Promise.resolve());
sandbox.stub(base, 'emit');
base.options.location = {};
base.options.viewer = {
pauseRequireJS: true
};
});

it('should create an asset URL and load the relevant stylesheets and scripts', () => {
base.loadAssets();

const result = base.loadAssets();
expect(util.createAssetUrlCreator).to.be.calledWith(base.options.location);
expect(util.loadStylesheets).to.be.called;
expect(util.loadScripts).to.be.called;
expect(result).to.equal(promise);
});

it('should emit "assetsloaded" if requireJS is paused and the asset is third party', () => {
return base.loadAssets().then(() => {
expect(base.emit).to.be.calledWith('assetsloaded');
});
});

it('should not emit "assetsloaded" if we load one of our own assets', () => {
return base.loadAssets([], [], false).then(() => {
expect(base.emit).to.not.be.called;
});
});
});

Expand Down Expand Up @@ -798,7 +814,7 @@ describe('lib/viewers/BaseViewer', () => {

it('should load the annotations assets', () => {
base.loadAnnotator();
expect(base.loadAssets).to.be.calledWith(['annotations.js'], ['annotations.css']);
expect(base.loadAssets).to.be.calledWith(['annotations.js'], ['annotations.css'], false);
});
});

Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/box3d/image360/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ The 360 image viewer fires the following events
| navigate | The preview is shown for a given index | {object} file |
| reload | The preview reloads ||
| resize | The preview resizes | 1. {number} **height**: window height 2. {number} **width**: window width |
| assetsloaded | The viewer's third party assets have loaded ||


## Methods

Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/box3d/model3d/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ The Model3D viewer fires the following events
| navigate | The preview is shown for a given index | {object} file |
| reload | The preview reloads ||
| resize | The preview resizes | 1. {number} **height**: window height 2. {number} **width**: window width |
| assetsloaded | The viewer's third party assets have loaded ||


## Methods

Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/doc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ The document viewer fires the following events
| scrollend | The viewer stops scrolling | 1. {number} **scrollTop**: number of pixels scrolled from top of viewport 2. {number} **scrollLeft**: number of pixels scrolled from left of viewport |
| printsuccess | An attempt to print triggered successfully ||
| printsuccess | An attempt to print failed ||
| assetsloaded | The viewer's third party assets have loaded ||

## Methods

Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/image/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ The image viewer fires the following events
| rotate | The image rotates ||
| printsuccess | An attempt to print triggered successfully ||


## Methods

The following methods are available for the image viewer.
Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/media/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ The DASH viewer fires the following events
| autoplay | the autoplay option has been enabled/disabled | {boolean} new autoplay value|
| pause | The video pauses ||
| seeked | The video skips to a time | {number} time |
| assetsloaded | The viewer's third party assets have loaded ||


## Methods

Expand Down
3 changes: 2 additions & 1 deletion src/lib/viewers/swf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ The SWF viewer fires the following events
| notification | A notification is displayed ||
| navigate | The preview is shown for a given index | {object} file |
| reload | The preview reloads ||
| resize | The preview resizes | 1. {number} **height**: window height 2. {number} **width**: window width |
| resize | The preview resizes | 1. {number} **height**: window height 2. {number} **width**: window | width |
| assetsloaded | The viewer's third party assets have loaded ||
2 changes: 2 additions & 0 deletions src/lib/viewers/text/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ The CSV viewer fires the following events
| reload | The preview reloads ||
| resize | The preview resizes | 1. {number} **height**: window height 2. {number} **width**: window width |
| zoom | The preview zooms in or out | 1. {number} **zoom**: new zoom value 2. {boolean} **canZoomIn**: true if the viewer can zoom in more 3. {boolean} **canZoomOut**: true if the viewer can zoom out more |
| assetsloaded | The viewer's third party assets have loaded ||


## Methods

Expand Down

0 comments on commit 3f26c81

Please sign in to comment.