🐛 Fixed Code Injection input fields not being clickable

no issue
- lazy loaded scripts such as the CodeMirror asset used on the Code Injection screen could throw errors such as `TypeError: Cannot set property 'modeOption' of undefined`
- this was caused by "loading" promise returned from the `lazyLoader` service returning as soon as the network request finished which can be before the loaded script has been parsed and run meaning any processing occurring after the promise returns could be depending on unloaded code
- switched the lazyLoader service's loading mechanism from an ajax fetch to insertion of a `<script>` tag which can have `load` event attached which _will_ return after parsing/loading has completed
This commit is contained in:
Kevin Ansfield 2019-02-11 21:07:51 +00:00
parent 346fef5947
commit e7c3b1b0e3
3 changed files with 43 additions and 33 deletions

View File

@ -1,6 +1,5 @@
/* global CodeMirror */
import Component from '@ember/component';
import RSVP from 'rsvp';
import boundOneWay from 'ghost-admin/utils/bound-one-way';
import {assign} from '@ember/polyfills';
import {bind, once, scheduleOnce} from '@ember/runloop';
@ -63,10 +62,7 @@ const CmEditorComponent = Component.extend({
initCodeMirror: task(function* () {
let loader = this.get('lazyLoader');
yield RSVP.all([
loader.loadScript('codemirror', 'assets/codemirror/codemirror.js')
]);
yield loader.loadScript('codemirror', 'assets/codemirror/codemirror.js');
scheduleOnce('afterRender', this, this._initCodeMirror);
}),

View File

@ -1,4 +1,3 @@
import $ from 'jquery';
import RSVP from 'rsvp';
import Service, {inject as service} from '@ember/service';
import config from 'ghost-admin/config/environment';
@ -22,31 +21,41 @@ export default Service.extend({
},
loadScript(key, url) {
if (this.get('testing')) {
if (this.testing) {
return RSVP.resolve();
}
if (this.get(`scriptPromises.${key}`)) {
// Script is already loaded/in the process of being loaded,
// so return that promise
return this.get(`scriptPromises.${key}`);
if (this.scriptPromises[key]) {
return this.scriptPromises[key];
}
let ajax = this.get('ajax');
let adminRoot = this.get('ghostPaths.adminRoot');
let scriptPromise = new RSVP.Promise((resolve, reject) => {
let {adminRoot} = this.ghostPaths;
let scriptPromise = ajax.request(`${adminRoot}${url}`, {
dataType: 'script',
cache: true
let script = document.createElement('script');
script.type = 'text/javascript';
script.async = true;
script.src = `${adminRoot}${url}`;
let el = document.getElementsByTagName('script')[0];
el.parentNode.insertBefore(script, el);
script.addEventListener('load', () => {
resolve();
});
script.addEventListener('error', () => {
reject(new Error(`${url} failed to load`));
});
});
this.set(`scriptPromises.${key}`, scriptPromise);
this.scriptPromises[key] = scriptPromise;
return scriptPromise;
},
loadStyle(key, url, alternate = false) {
if (this.get('testing') || $(`#${key}-styles`).length) {
if (this.testing || document.querySelector(`#${key}-styles`)) {
return RSVP.resolve();
}
@ -54,7 +63,7 @@ export default Service.extend({
let link = document.createElement('link');
link.id = `${key}-styles`;
link.rel = alternate ? 'alternate stylesheet' : 'stylesheet';
link.href = `${this.get('ghostPaths.adminRoot')}${url}`;
link.href = `${this.ghostPaths.adminRoot}${url}`;
link.onload = () => {
if (alternate) {
// If stylesheet is alternate and we disable the stylesheet before injecting into the DOM,
@ -69,7 +78,7 @@ export default Service.extend({
link.title = key;
}
$('head').append($(link));
document.querySelector('head').appendChild(link);
});
}
});

View File

@ -6,6 +6,7 @@ import {setupTest} from 'ember-mocha';
describe('Integration: Service: lazy-loader', function () {
setupTest('service:lazy-loader', {integration: true});
let server;
let ghostPaths = {
adminRoot: '/assets/'
@ -19,28 +20,32 @@ describe('Integration: Service: lazy-loader', function () {
server.shutdown();
});
it('loads a script correctly and only once', function () {
it('loads a script correctly and only once', async function () {
let subject = this.subject({
ghostPaths,
scriptPromises: {},
testing: false
});
server.get('/assets/test.js', function ({requestHeaders}) {
expect(requestHeaders.Accept).to.match(/text\/javascript/);
// first load should add script element
await subject.loadScript('test', 'lazy-test.js')
.then(() => {})
.catch(() => {});
return [200, {'Content-Type': 'text/javascript'}, 'window.testLoadScript = \'testvalue\''];
});
expect(
document.querySelectorAll('script[src="/assets/lazy-test.js"]').length,
'no of script tags on first load'
).to.equal(1);
return subject.loadScript('test-script', 'test.js').then(() => {
expect(subject.get('scriptPromises.test-script')).to.exist;
expect(window.testLoadScript).to.equal('testvalue');
expect(server.handlers[0].numberOfCalls).to.equal(1);
// second load should not add another script element
await subject.loadScript('test', '/assets/lazy-test.js')
.then(() => { })
.catch(() => { });
return subject.loadScript('test-script', 'test.js');
}).then(() => {
expect(server.handlers[0].numberOfCalls).to.equal(1);
});
expect(
document.querySelectorAll('script[src="/assets/lazy-test.js"]').length,
'no of script tags on second load'
).to.equal(1);
});
it('loads styles correctly', function () {