opendev/gerrit
Code Issues Proposed changes

Generalize library loader

Browse Source 
Until recently, the only side-loaded resource was the HLJS library for
computing syntax highlighting. For this task, the gr-syntax-lib-loader
provided an interface to load the library whether or not PG is being
served from a CDN.

With this change, the component is refactored to allow loading resources
other than the syntax library. A method is added for loading the
"dark-theme" document independently of whether a CDN is configured.

Also, some documentation comments are added to the existing methods.

Change-Id: I9891539cd4cf76ac0fe430ff3988e3a9dfbb0ca3
This commit is contained in:
Wyatt Allen
2018-05-21 10:50:20 -07:00
parent 691423248d
commit e9bf51941a
9 changed files with 91 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.html
View File

@@ -15,11 +15,11 @@ See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../gr-syntax-lib-loader/gr-syntax-lib-loader.html">
<link rel="import" href="../../shared/gr-lib-loader/gr-lib-loader.html">
<dom-module id="gr-syntax-layer">
<template>
<gr-syntax-lib-loader id="libLoader"></gr-syntax-lib-loader>
<gr-lib-loader id="libLoader"></gr-lib-loader>
</template>
<script src="../gr-diff/gr-diff-line.js"></script>
<script src="../gr-diff-highlight/gr-annotation.js"></script>

2
polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
View File

@@ -442,7 +442,7 @@
},
_loadHLJS() {
return this.$.libLoader.get().then(hljs => {
return this.$.libLoader.getHLJS().then(hljs => {
this._hljs = hljs;
});
},

2
polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer_test.html
View File

@@ -185,7 +185,7 @@ limitations under the License.
const mockHLJS = getMockHLJS();
const highlightSpy = sinon.spy(mockHLJS, 'highlight');
sandbox.stub(element.$.libLoader, 'get',
sandbox.stub(element.$.libLoader, 'getHLJS',
() => { return Promise.resolve(mockHLJS); });
const processNextSpy = sandbox.spy(element, '_processNextLine');
const processPromise = element.process();

2
polygerrit-ui/app/elements/gr-app.html
View File

@@ -56,6 +56,7 @@ limitations under the License.
<link rel="import" href="./settings/gr-registration-dialog/gr-registration-dialog.html">
<link rel="import" href="./settings/gr-settings-view/gr-settings-view.html">
<link rel="import" href="./shared/gr-fixed-panel/gr-fixed-panel.html">
<link rel="import" href="./shared/gr-lib-loader/gr-lib-loader.html">
<link rel="import" href="./shared/gr-rest-api-interface/gr-rest-api-interface.html">
<script src="../scripts/util.js"></script>
@@ -229,6 +230,7 @@ limitations under the License.
<gr-plugin-host id="plugins"
config="[[_serverConfig]]">
</gr-plugin-host>
<gr-lib-loader id="libLoader"></gr-lib-loader>
<gr-external-style id="externalStyle" name="app-theme"></gr-external-style>
</template>
<script src="gr-app.js" crossorigin="anonymous"></script>

2
polygerrit-ui/app/elements/gr-app.js
View File

@@ -128,7 +128,7 @@
});
if (window.localStorage.getItem('dark-theme')) {
this.importHref('../styles/themes/dark-theme.html');
this.$.libLoader.loadDarkTheme();
}
// Note: this is evaluated here to ensure that it only happens after the

4
polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader.html → polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.html
View File

@@ -16,6 +16,6 @@ limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<dom-module id="gr-syntax-lib-loader">
<script src="gr-syntax-lib-loader.js"></script>
<dom-module id="gr-lib-loader">
<script src="gr-lib-loader.js"></script>
</dom-module>

81
polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader.js → polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
View File

@@ -18,13 +18,14 @@
'use strict';
const HLJS_PATH = 'bower_components/highlightjs/highlight.min.js';
const DARK_THEME_PATH = 'styles/themes/dark-theme.html';
const LIB_ROOT_PATTERN = /(.+\/)elements\/gr-app\.html/;
Polymer({
is: 'gr-syntax-lib-loader',
is: 'gr-lib-loader',
properties: {
_state: {
_hljsState: {
type: Object,
// NOTE: intended singleton.
@@ -36,7 +37,13 @@
},
},
get() {
/**
* Get the HLJS library. Returns a promise that resolves with a reference to
* the library after it's been loaded. The promise resolves immediately if
* it's already been loaded.
* @return {!Promise<Object>}
*/
getHLJS() {
return new Promise((resolve, reject) => {
// If the lib is totally loaded, resolve immediately.
if (this._getHighlightLib()) {
@@ -45,55 +52,87 @@
}
// If the library is not currently being loaded, then start loading it.
if (!this._state.loading) {
this._state.loading = true;
this._loadHLJS().then(this._onLibLoaded.bind(this)).catch(reject);
if (!this._hljsState.loading) {
this._hljsState.loading = true;
this._loadScript(this._getHLJSUrl())
.then(this._onHLJSLibLoaded.bind(this)).catch(reject);
}
this._state.callbacks.push(resolve);
this._hljsState.callbacks.push(resolve);
});
},
_onLibLoaded() {
const lib = this._getHighlightLib();
this._state.loading = false;
for (const cb of this._state.callbacks) {
cb(lib);
}
this._state.callbacks = [];
/**
* Load and apply the dark theme document.
*/
loadDarkTheme() {
this.importHref(this._getLibRoot() + DARK_THEME_PATH);
},
/**
* Execute callbacks awaiting the HLJS lib load.
*/
_onHLJSLibLoaded() {
const lib = this._getHighlightLib();
this._hljsState.loading = false;
for (const cb of this._hljsState.callbacks) {
cb(lib);
}
this._hljsState.callbacks = [];
},
/**
* Get the HLJS library, assuming it has been loaded. Configure the library
* if it hasn't already been configured.
* @return {!Object}
*/
_getHighlightLib() {
const lib = window.hljs;
if (lib && !this._state.configured) {
this._state.configured = true;
if (lib && !this._hljsState.configured) {
this._hljsState.configured = true;
lib.configure({classPrefix: 'gr-diff gr-syntax gr-syntax-'});
}
return lib;
},
/**
* Get the resource path used to load the application. If the application
* was loaded through a CDN, then this will be the path to CDN resources.
* @return {string}
*/
_getLibRoot() {
if (this._cachedLibRoot) { return this._cachedLibRoot; }
const appLink = document.head
.querySelector('link[rel=import][href$="gr-app.html"]');
if (!appLink) { return null; }
if (!appLink) { throw new Error('Could not find application link'); }
return this._cachedLibRoot = appLink
this._cachedLibRoot = appLink
.href
.match(LIB_ROOT_PATTERN)[1];
if (!this._cachedLibRoot) {
throw new Error('Could not extract lib root');
}
return this._cachedLibRoot;
},
_cachedLibRoot: null,
_loadHLJS() {
/**
* Load and execute a JS file from the lib root.
* @param {string} src The path to the JS file without the lib root.
* @return {Promise} a promise that resolves when the script's onload
* executes.
*/
_loadScript(src) {
return new Promise((resolve, reject) => {
const script = document.createElement('script');
const src = this._getHLJSUrl();
if (!src) {
reject(new Error('Unable to load blank HLJS url.'));
reject(new Error('Unable to load blank script url.'));
return;
}

44
polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html → polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader_test.html
View File

@@ -17,64 +17,67 @@ limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-syntax-lib-loader</title>
<title>gr-lib-loader</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
<link rel="import" href="gr-syntax-lib-loader.html">
<link rel="import" href="gr-lib-loader.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
<gr-syntax-lib-loader></gr-syntax-lib-loader>
<gr-lib-loader></gr-lib-loader>
</template>
</test-fixture>
<script>
suite('gr-syntax-lib-loader tests', () => {
suite('gr-lib-loader tests', () => {
let sandbox;
let element;
let resolveLoad;
let loadStub;
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
loadStub = sinon.stub(element, '_loadHLJS', () =>
loadStub = sandbox.stub(element, '_loadScript', () =>
new Promise(resolve => resolveLoad = resolve)
);
// Assert preconditions:
assert.isFalse(element._state.loading);
assert.isFalse(element._hljsState.loading);
});
teardown(() => {
if (window.hljs) {
delete window.hljs;
}
loadStub.restore();
sandbox.restore();
// Because the element state is a singleton, clean it up.
element._state.configured = false;
element._state.loading = false;
element._state.callbacks = [];
element._hljsState.configured = false;
element._hljsState.loading = false;
element._hljsState.callbacks = [];
});
test('only load once', done => {
sandbox.stub(element, '_getHLJSUrl').returns('');
const firstCallHandler = sinon.stub();
element.get().then(firstCallHandler);
element.getHLJS().then(firstCallHandler);
// It should now be in the loading state.
assert.isTrue(loadStub.called);
assert.isTrue(element._state.loading);
assert.isTrue(element._hljsState.loading);
assert.isFalse(firstCallHandler.called);
const secondCallHandler = sinon.stub();
element.get().then(secondCallHandler);
element.getHLJS().then(secondCallHandler);
// No change in state.
assert.isTrue(element._state.loading);
assert.isTrue(element._hljsState.loading);
assert.isFalse(firstCallHandler.called);
assert.isFalse(secondCallHandler.called);
@@ -82,7 +85,7 @@ limitations under the License.
resolveLoad();
flush(() => {
// The state should be loaded and both handlers called.
assert.isFalse(element._state.loading);
assert.isFalse(element._hljsState.loading);
assert.isTrue(firstCallHandler.called);
assert.isTrue(secondCallHandler.called);
done();
@@ -105,7 +108,7 @@ limitations under the License.
test('returns hljs', done => {
const firstCallHandler = sinon.stub();
element.get().then(firstCallHandler);
element.getHLJS().then(firstCallHandler);
flush(() => {
assert.isTrue(firstCallHandler.called);
assert.isTrue(firstCallHandler.calledWith(hljsStub));
@@ -114,7 +117,7 @@ limitations under the License.
});
test('configures hljs', done => {
element.get().then(() => {
element.getHLJS().then(() => {
assert.isTrue(window.hljs.configure.calledOnce);
done();
});
@@ -123,15 +126,10 @@ limitations under the License.
suite('_getHLJSUrl', () => {
suite('checking _getLibRoot', () => {
let libRootStub;
let root;
setup(() => {
libRootStub = sinon.stub(element, '_getLibRoot', () => root);
});
teardown(() => {
libRootStub.restore();
sandbox.stub(element, '_getLibRoot', () => root);
});
test('with no root', () => {

2
polygerrit-ui/app/test/index.html
View File

@@ -112,7 +112,6 @@ limitations under the License.
'diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html',
'diff/gr-selection-action-box/gr-selection-action-box_test.html',
'diff/gr-syntax-layer/gr-syntax-layer_test.html',
'diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html',
'edit/gr-default-editor/gr-default-editor_test.html',
'edit/gr-edit-controls/gr-edit-controls_test.html',
'edit/gr-edit-file-controls/gr-edit-file-controls_test.html',
@@ -165,6 +164,7 @@ limitations under the License.
'shared/gr-js-api-interface/gr-plugin-endpoints_test.html',
'shared/gr-js-api-interface/gr-plugin-rest-api_test.html',
'shared/gr-fixed-panel/gr-fixed-panel_test.html',
'shared/gr-lib-loader/gr-lib-loader_test.html',
'shared/gr-limited-text/gr-limited-text_test.html',
'shared/gr-linked-chip/gr-linked-chip_test.html',
'shared/gr-linked-text/gr-linked-text_test.html',