fedwiki / wiki-server

Federated Wiki client and server in Node.js
Other
153 stars 35 forks source link

inefficient (?) load of prettify.js/css #49

Closed espinielli closed 8 years ago

espinielli commented 10 years ago

The use of code plugin inserts a

<style type="text/css">@import url("/plugins/code/prettify.css")</style>

line for every code snippet in the page. This sounds inefficient in principle, but for this specific plugin not a problem.

paul90 commented 10 years ago

Sounds like an issue across the wiki-client and plug-ins.

Think we need to add a function to the wiki-client that will add a requested resource, if it hasn't been added already. And, for the client to call that function when they want to add a resource.

Also need to check that the plug-in code is only getting loaded the once.

WardCunningham commented 10 years ago

I've noticed that pages with many code items render slowly. I had guessed that it was dom manipulation or something else like that. It would be good to check this from the browser's inspector to find out what is really going on.

WardCunningham commented 10 years ago

I created a simple page with thirty lines in a code plugin. Watching the network traffic in chrome on a shift-reload I saw that prettify.css was still loaded from cache. Strangely, prettify.js was loaded twice to render one code item. The prettify.js load included cache-busting random parameters so it really did load it twice.

I added a second code item to the page, reloaded, saw prettify.css still only loaded once, from cache, but prettify.js loaded four times, each time with a different parameter which I now recognize as a millisecond clock.

I added six more items for a total of eight code items. Rendering this page loads prettify.js 16 times. Yikes.

screen shot 2014-01-15 at 8 02 04 am

I haven't looked a code yet. But I'd expect there is something foolish in the code plugin, or in the utility that it calls to do the loading. I suggest we wait on working this issue further until our new code organization and build processes are stable. Then we should look into best practice for dynamic loading javascript and scoping the css that goes with it.

paul90 commented 10 years ago

Interesting that prettify.css is only getting downloaded the once, looking in the DOM it is injected into the head multiple times, but I guess the browser has the sense to only fetch it the once.

A quick search gives this :point_right: How to use Javascript to check and load CSS if not loaded?.

In wiki-plugin-code, the load is

  load = (callback) ->
    wiki.getScript '/plugins/code/prettify.js', callback
    $('<style type="text/css"></style>')
      .html('@import url("/plugins/code/prettify.css")')
      .appendTo("head");

wiki.getScript is defined over in wiki-client/lib/plugin.coffee. And, from a quick step through, it looks as if the problem is in

scripts = {}
getScript = wiki.getScript = (url, callback = () ->) ->
  if scripts[url]?              
    callback()
  else
    $.getScript(url)
      .done ->
        scripts[url] = true
        callback()
      .fail ->
        callback()

the scripts[url]? always appears to be false, so the script is loaded multiple times.

think this should be something more like

scripts = []
getScript = wiki.getScript = (url, callback = () ->) ->
  if url in scripts               
    callback()
  else
    $.getScript(url)
      .done ->
        scripts.push url
        callback()
      .fail ->
        callback()

will probably try this in the morning.

paul90 commented 10 years ago

I added a second code item to the page, reloaded, saw prettify.css still only loaded once, from cache, but prettify.js loaded four times, each time with a different parameter which I now recognize as a millisecond clock.

See Caching Responses on jQuery.getScript, probably setting cache to true is an option.

But, there remains the question of why it is loaded twice rather than just the once for each code block.

paul90 commented 10 years ago

Digging a bit deeper, looking at why prettify.js is getting loaded twice - writing the url and callback to the console when wiki.getScript is called. We see

capture

in wiki-plugin-code/code.coffee the load is getting called twice, once for the emit, and again for the bind. Both are calling wiki.getScript - so, the script is getting loaded twice.

Will probably add the code so that jQuery.getScript will cache the load, but as both are happening in parallel I don't expect it to solve this problem.

Adding code so that the getScript call is cache results in the script just getting loaded the once, extract from the server logs below.

capture

WardCunningham commented 10 years ago

I feel an architectural weakness in our approach to dynamic loading of both plugins and the resources plugins need. We've lived with this for some time. I'd be happy to see these weaknesses patched or even ignored while we work on bigger problems. That said, this is some suggestions I've received from wise people regarding loading plugins:

What we have now is the result of taking lots of advice from Stack Overflow and then moving on once things seemed to work. This is a good way to get a feel for what you really need but doesn't lead to software well aligned with our dynamic environment.

I'd like to revisit dynamic loading once we have some more experience writing and maintaining plugins in our new repo configuration. This could include a rethink of how the core interacts with plugins.

paul90 commented 10 years ago

I'd be happy to see these weaknesses patched or even ignored while we work on bigger problems.

In some respects, this issue has help with the refactor - I have been keeping notes on the steps I have been following.

I propose that we only apply the changes for the refactored node version (the ruby version will pick-up the change when it is refactored to use the new repositories).