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

importHref loads imports, async. Fixes #3113 #3114

Merged
merged 1 commit into from
Dec 10, 2015
Merged

importHref loads imports, async. Fixes #3113 #3114

merged 1 commit into from
Dec 10, 2015

Conversation

ebidel
Copy link
Contributor

@ebidel ebidel commented Nov 30, 2015

This changes the default behavior of importHref to load imports async, taking them out of the rendering path. Witnessing this is a a couple of apps we're working on, not doing so causes layout flashes as the import is added to the DOM and loads.

Adds an optional async arg to importHref. Defaults to true for perf and similar to xhr.

R: @kevinpschaaf @sorvell @azakus

@ebidel
Copy link
Contributor Author

ebidel commented Dec 9, 2015

Should we use this or make the default false? Async by default would be preferrable though!

@TimvdLippe
Copy link
Contributor

👍

* @return {HTMLLinkElement} The link element for the URL to be loaded.
*/
importHref: function(href, onload, onerror) {
importHref: function(href, onload, onerror, optAsync) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd say we make this default to false for back-compat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dfreedm
Copy link
Member

dfreedm commented Dec 10, 2015

LGTM with nit

@ebidel
Copy link
Contributor Author

ebidel commented Dec 10, 2015

PTAL

var l = document.createElement('link');
l.rel = 'import';
l.href = href;

optAsync = typeof optAsync !== 'undefined' ? optAsync : false;
Copy link
Member

Choose a reason for hiding this comment

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

This can just be optAsync = Boolean(optAsync) since the default is false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dfreedm
Copy link
Member

dfreedm commented Dec 10, 2015

Last nit, then I can merge :)

@ebidel
Copy link
Contributor Author

ebidel commented Dec 10, 2015

PTAL

dfreedm added a commit that referenced this pull request Dec 10, 2015
importHref loads imports, async. Fixes #3113
@dfreedm dfreedm merged commit 9066a48 into master Dec 10, 2015
@dfreedm dfreedm deleted the 3113 branch December 10, 2015 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants