Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the script tag after-render #48

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions addon/loaders/css.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import RSVP from 'rsvp';
import { createLoadElement, nodeLoader } from './utilities';
import { scheduleWork } from './scheduler';

/**
* Default loader function for CSS assets. Loads them by inserting a link tag
Expand All @@ -19,34 +20,46 @@ export default nodeLoader(function css(uri) {
return resolve();
}

// Try using the default onload/onerror handlers...
const link = createLoadElement('link', resolve, reject);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move this declaration outside the block where it is initialized?

scheduleWork(() => {
let link;
try {
// Try using the default onload/onerror handlers...
link = createLoadElement('link', resolve, reject);

link.rel = 'stylesheet';
link.href = uri;
link.rel = 'stylesheet';
link.href = uri;

document.head.appendChild(link);
document.head.appendChild(link);

// In case the browser doesn't fire onload/onerror events, we poll the
// the list of stylesheets to see when it loads...
function checkSheetLoad() {
const resolvedHref = link.href;
const stylesheets = document.styleSheets;
let i = stylesheets.length;
// In case the browser doesn't fire onload/onerror events, we poll the
// the list of stylesheets to see when it loads...

while (i--) {
const sheet = stylesheets[i];
if (sheet.href === resolvedHref) {
// Unfortunately we have no way of knowing if the load was
// successful or not, so we always resolve.
setTimeout(resolve);
return;
}
setTimeout(checkSheetLoad);
} catch(reason) {
reject(reason);
}

setTimeout(checkSheetLoad);
}
function checkSheetLoad() {
try {
const resolvedHref = link.href;
const stylesheets = document.styleSheets;
let i = stylesheets.length;

while (i--) {
const sheet = stylesheets[i];
if (sheet.href === resolvedHref) {
// Unfortunately we have no way of knowing if the load was
// successful or not, so we always resolve.
setTimeout(resolve);
return;
}
}

setTimeout(checkSheetLoad);
setTimeout(checkSheetLoad);
} catch(reason) {
reject(reason);
}
}
});
});
});
27 changes: 23 additions & 4 deletions addon/loaders/js.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import RSVP from 'rsvp';
import { createLoadElement, nodeLoader } from './utilities';
import { scheduleWork } from './scheduler';

/**
* Default loader function for JS assets. Loads them by inserting a script tag
Expand All @@ -15,11 +16,29 @@ export default nodeLoader(function js(uri) {
return resolve();
}

const script = createLoadElement('script', resolve, reject);
// DOM mutation should be batched, this indirection enables this batching.
// By default, it will schedule work on the next afterRender queue. But can
// be configured for further control via.
//
// ```js
// import { setScheduler } from 'ember-asset-loader/scheduler';
//
// setScheduler(function(work /* work is a callback */) {
// someScheduler.scheduleWork(work);
// });
// ```
//
scheduleWork(() => {
try {
const script = createLoadElement('script', resolve, reject);

script.src = uri;
script.async = false;
script.src = uri;
script.async = false;

document.head.appendChild(script);
document.head.appendChild(script);
} catch(e) {
reject(e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What error would we expect to happen here?

Copy link
Contributor Author

@stefanpenner stefanpenner May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none, this is to handle the exceptional cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also maintains existing behavior. Currently if any of those lines error, the promise constructor will catch them. As this PR moves them to be async, we must manage that ourselves.

}
});
});
});
12 changes: 12 additions & 0 deletions addon/loaders/scheduler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Ember from 'ember';
export const defaultScheduler = (work) => Ember.run.schedule('afterRender', work);

let scheduler = defaultScheduler;

export function setScheduler(_scheduler) {
scheduler = _scheduler;
}

export function scheduleWork(work) {
Ember.run.join(scheduler, work);
}
19 changes: 9 additions & 10 deletions tests/unit/services/asset-loader-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ function noop() {}
function shouldNotHappen(assert) {
return () => assert.ok(false, 'callback should not happen');
}
function shouldHappen(assert) {
return () => assert.ok(true, 'callback should happen');
}

moduleFor('service:asset-loader', 'Unit | Service | asset-loader');

Expand Down Expand Up @@ -252,12 +249,12 @@ test('loadAsset() - retrying a load twice returns the same promise', function(as
});

test('loadAsset() - js - handles successful load', function(assert) {
assert.expect(1);
assert.expect(0);

const service = this.subject();
const asset = { type: 'js', uri: '/unit-test.js' };

return service.loadAsset(asset).then(shouldHappen(assert), shouldNotHappen(assert));
return service.loadAsset(asset);
});

test('loadAsset() - js - handles failed load', function(assert) {
Expand All @@ -266,7 +263,7 @@ test('loadAsset() - js - handles failed load', function(assert) {
const service = this.subject();
const asset = { type: 'js', uri: '/unit-test.jsss' };

return service.loadAsset(asset).then(shouldNotHappen(assert), shouldHappen(assert));
return service.loadAsset(asset).catch(() => assert.ok(true, 'loadAsset should reject'));
});

test('loadAsset() - js - does not insert additional script tag if asset is in DOM already', function(assert) {
Expand Down Expand Up @@ -301,12 +298,12 @@ test('loadAsset() - js - sets async false to try to guarantee execution order',
});

test('loadAsset() - css - handles successful load', function(assert) {
assert.expect(1);
assert.expect(0);

const service = this.subject();
const asset = { type: 'css', uri: '/unit-test.css' };

return service.loadAsset(asset).then(shouldHappen(assert), shouldNotHappen(assert));
return service.loadAsset(asset);
});

test('loadAsset() - css - handles failed load', function(assert) {
Expand All @@ -319,9 +316,11 @@ test('loadAsset() - css - handles failed load', function(assert) {
// non-Chrome browsers to either resolve or reject (they should do something).
var isChrome = !!window.chrome && window.navigator.vendor === 'Google Inc.';
if (isChrome) {
return service.loadAsset(asset).then(shouldNotHappen(assert), shouldHappen(assert));
return service.loadAsset(asset).catch(() => assert.ok(true, 'loadAsset should reject'));
} else {
return service.loadAsset(asset).then(shouldHappen(assert), shouldHappen(assert));
return service.loadAsset(asset)
.then(() => assert.ok(true, 'loadAsset may resolve'))
.catch(() => assert.ok(true, 'loadAsset may reject'));
}
});

Expand Down
Loading