Better js #161

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

loxK commented Sep 7, 2011

No description provided.

Contributor

divergeinfinity commented Nov 7, 2011

It can't be auto-merged after 2 months and can't be automerged, plus I'm not sure if this is actually required?

Contributor

loxK commented Nov 7, 2011

For sure now it is too late.

But I hope you kept that on a roadmap somewhere, because Jigoshop doesn't load its scripts in a clever way (understand where and when it is needed, which is a bad practice),

The first step is to make them lighter. Replacing jQuery with its $ alias is a first step and must be done.

The second step should be to load the scripts when and where needed, that is the purpose of the script loader. That makes a difference for WP power users (like web developer). Good quality and well written plugins do not overload your webite with unneeded scripts everywhere....

Then it will be needed to separate scripts used outside Jigoshop pages (eg. widgets scripts).

Contributor

studioromeo commented Nov 7, 2011

@loxK It's never too late, git conflicts can be worked out & often do that :)
I'm not so sure on either of your views, for starters wp_enqueue_script should be called with the wp_enqueue_scripts hook not the init as it's too early. After thinking about it..we have modernizr...which has yepnope...which means we can bring in scripts & styles on the fly & also provide resource fallbacks so we can make use of Googles AJAX librarys but still hold a local copy.

Another benefit is it keeps javascript with the javascript, I'm weird and like things like that :P

What do you guys think? Could probably do with some speed tests I guess.

Contributor

loxK commented Nov 7, 2011

Loading at init or at wp_enqueue_script action hook is not a rule, it needs a case per case approach. Also the good practice that any good web developer is aware of, is that Javascripts should be loaded at the end of the page (wp_footer)

Concerning modernizr, I don't think it is a good idea, jigoshop should load only a minimal amount of packed JS. But if it doesn't increase pages loading, why not, but I doubt it is the case. I understand that themes can make use of it, not Jigoshop.

Yepnote is a nice thing, I haven't looked deeply into it, but I think that it does initiate one request per script to load which slows down things.

Contributor

studioromeo commented Nov 7, 2011

@loxK I'd be interested to hear the reasoning using init over wp_enqueue_scripts.

Agreed that as a rule we should try and keep the js down...that being said modernizr at least from my experience has become almost a standard tool (like jQuery) in creating cross platform themes. With this in mind it incurs 0 page load increase because chances are

1 it's already cached in the users browser
2 or its all ready being used by a theme.

Yep nope is included in the package along with respond.js (I think), and of course html5shiv so it has all the components needed.

Not sure what you mean by slowing down things, it doesn't inherit the resource blocking you get normally. The only thing I would say is we shouldn't use this to load external libraries, as in jQuery ect because of multiple versions being used ect

Contributor

loxK commented Nov 7, 2011

Well wp_enqueue_scripts is the hook to use. I think its name says it all. But in some case you may want to be totally sure a script is queued before any other, in that case (the only one I see right now) init can be used.

I still think modernizr shouldn't be part of Jigoshop. I don't see any reason for that. And it is overloading websites using it because, even if cached, a request is done and because it does work on the DOM, and thus can slow down the websites using it on some low end devices. The inclusion of it should be left to themes, according to me.

Concerning Yepnote , my thought is that you plan to split Jigoshop javascripts into a lot of small js files: one for the cart, one for each widgets, one for the product list and maybe more. In that case, a request per script will be done, which is bad for page loading speed: HTTP requests are a major performance bottleneck for web page, less requests per page = faster loading time (that is why CSS sprites are good for page loading, bigger image, but less requests)

Concerning Jquery UI that Jigoshop loads from google, it is a bad idea to me. We should stick to the version bundled in wordpress. Then if one theme wants a more recent version, it is up to its developer to make use of a more recent version.

One very important thing I already talked about is loading jigoshop scripts in the footer.

Contributor

studioromeo commented Nov 7, 2011

Ah I see, isn't that what the $deps attribute is for?

With regards to speed impact, from the man himself:

The fastest page is a blank one. Everything you add to a page slows it down.

That said, I can speak to the performance of Modernizr, which I've tuned to complete all its tests in under 20ms.
Modernizr only runs once, so it has no impact on the runtime performance. (Except if you try to print in oldIE, where
Modernizr and the html5shiv intercept that and fix it)

-Paul Irish

So yeah a little but not much. I'm just looking ahead most theme authors will probably use modernizr (hell of an assumption i know but its become standard practice to feature detect) Yepnopes included with this so for me it seems like a pretty good deal

Yepnope: Not at all, my plan was to simply swap your script loader for an asynchronous one which squeezes out more performance by removing the file blocking issue. How many scripts are split? Depends on the whats fastest doesn't it.

While I dislike the idea of using local scripts when there is a perfectly good CDN out there, WordPress wants us to use local scripts so I'm in favour.

Scripts in footer? Hell yeah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment