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

Feat(w3c/style): preload for css, scripts (see #663) #838

Merged
merged 18 commits into from Jun 28, 2016

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Jun 23, 2016

This mostly helps github hosted editor's drafts.

@yoavweiss, wdyt?

@marcoscaceres marcoscaceres force-pushed the preload-fixup branch 2 times, most recently from 7f95f6a to ebb3a00 Compare June 23, 2016 01:39
var preconnectLink = document.createElement("link");
preconnectLink.rel = "preconnect";
preconnectLink.href = "https://www.w3.org";
preconnectLink.crossorigin = true;
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be "crossOrigin"

Copy link
Member Author

Choose a reason for hiding this comment

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

and = "anonymous";

@marcoscaceres marcoscaceres force-pushed the preload-fixup branch 2 times, most recently from e3b62d9 to ede9076 Compare June 23, 2016 14:14
@marcoscaceres marcoscaceres force-pushed the preload-fixup branch 2 times, most recently from 513d117 to 4201fd5 Compare June 24, 2016 02:24
@@ -10,8 +10,6 @@ define(
function(utils, pubsubhub) {
function attachFixupScript(doc, version){
var script = doc.createElement("script");
script.async = true;
script.defer = true;
Copy link
Member Author

Choose a reason for hiding this comment

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

So, my reading is that because this script is pre-loaded, we don't need to defer it. And as it's already at the end of the document (and added after processing), async won't matter.

@marcoscaceres
Copy link
Member Author

Before:
screenshot 2016-06-25 19 13 03

After:
screenshot 2016-06-25 19 15 11

😻

Important where base.css comes in now.

We are still getting like 4-5 repaints tho, which really sucks.

@marcoscaceres
Copy link
Member Author

Made a couple more performance improvements... main bottleneck seems to be handlebars tho... once we compile the templates, we should see maybe another ±10% gain. It's noticeably better now tho. Way less thrashing.

@halindrome
Copy link
Contributor

I sort of forgot that there was handlebars under the covers. I wonder if
other organizations that use ReSpec have their own templates that are
getting populated? Also, is the handlebars functionality available for the
content of a document? In other words, could I use handlebars in my specs
to do value substitution at doc generation time?

On Sat, Jun 25, 2016 at 5:25 AM, Marcos Cáceres notifications@github.com
wrote:

Made a couple more performance improvements... main bottleneck seems to be
handlebars tho... once we compile the templates, we should see maybe
another ±10% gain. It's noticeably better now tho. Way less thrashing.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#838 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAfx8JWJFBcQf3dj5nsj9oRPN7k74PpNks5qPQIYgaJpZM4I8Xwh
.

Shane McCarron
halindrome@gmail.com

@marcoscaceres
Copy link
Member Author

On 26 Jun 2016, at 2:35 AM, Shane McCarron notifications@github.com wrote:

I sort of forgot that there was handlebars under the covers. I wonder if
other organizations that use ReSpec have their own templates that are
getting populated?

It's likely, but given their complexity: odds are low.

Also, is the handlebars functionality available for the
content of a document?

I don't think so. Only should be in tmpl.js.

In other words, could I use handlebars in my specs
to do value substitution at doc generation time?

Only if you require() it.

On Sat, Jun 25, 2016 at 5:25 AM, Marcos Cáceres notifications@github.com
wrote:

Made a couple more performance improvements... main bottleneck seems to be
handlebars tho... once we compile the templates, we should see maybe
another ±10% gain. It's noticeably better now tho. Way less thrashing.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#838 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAfx8JWJFBcQf3dj5nsj9oRPN7k74PpNks5qPQIYgaJpZM4I8Xwh
.

Shane McCarron
halindrome@gmail.com

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@marcoscaceres
Copy link
Member Author

@halindrome, would you mind reviewing?

I was just looking for @yoavweiss feedback on approach and if he had any additional suggestions.

@@ -84,7 +102,54 @@ define(
}, +Infinity);
return (leftPad === +Infinity) ? 0 : leftPad;
},

/**
* Creates a link element to use as a resource hint.
Copy link
Member Author

Choose a reason for hiding this comment

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

Should say, "represents a resource hint" and maybe link to spec

@marcoscaceres
Copy link
Member Author

Did a couple of rounds of self review over last few days. Additional comments welcome.

@marcoscaceres marcoscaceres merged commit fce1b4a into develop Jun 28, 2016
@marcoscaceres marcoscaceres deleted the preload-fixup branch June 28, 2016 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants