Fix #156 #167

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@mcguffin

mcguffin commented Oct 3, 2014

No description provided.

@ccampbell

This comment has been minimized.

Show comment
Hide comment
@ccampbell

ccampbell Oct 5, 2014

Owner

Hey @mcguffin

Curious if you could provide a jsfiddle or some test page so I can understand the behavior you are expecting. I've been working a bit on rewriting Rainbow, and perhaps this is something that I should be including.

Thanks

Owner

ccampbell commented Oct 5, 2014

Hey @mcguffin

Curious if you could provide a jsfiddle or some test page so I can understand the behavior you are expecting. I've been working a bit on rewriting Rainbow, and perhaps this is something that I should be including.

Thanks

@mcguffin

This comment has been minimized.

Show comment
Hide comment
@mcguffin

mcguffin Oct 5, 2014

Hi @ccampbell
Just two minimalistic examples:
http://podpirate.org/wp-content/uploads/rainbow-works.html
http://podpirate.org/wp-content/uploads/rainbow-broken.html
The only difference between these two is that in the broken one the scripts are loaded async.
Thanks for being back :)

mcguffin commented Oct 5, 2014

Hi @ccampbell
Just two minimalistic examples:
http://podpirate.org/wp-content/uploads/rainbow-works.html
http://podpirate.org/wp-content/uploads/rainbow-broken.html
The only difference between these two is that in the broken one the scripts are loaded async.
Thanks for being back :)

@ccampbell

This comment has been minimized.

Show comment
Hide comment
@ccampbell

ccampbell Jul 3, 2016

Owner

I'm sorry for letting this project get a bit stale. I just released version 2.0, and I would like to make things better. I was just looking at this PR and the associated issue.

While your fix may allow Rainbow.color() to be called when rainbow is loaded asynchronously it doesn't guarantee that other script that depend on Rainbow will be loaded in the right order. For example if you have

<script async src="rainbow/js/rainbow.js"></script>
<script async src="rainbow/js/language/generic.js"></script>
<script async src="rainbow/js/language/php.js"></script>

If the generic.js or javascript.js come in before rainbow.js then nothing is going to work correctly since Rainbow will be undefined and also javascript depends on having the generic patterns already loaded.

I was reading here: http://www.html5rocks.com/en/tutorials/speed/script-loading/ and here: https://webkit.org/blog/1395/running-scripts-in-webkit/ about the different kinds of script loading and it sounds like the defer attribute is actually what you should be using here.

If you change your demo page to use defer then you will get the same benefit as async where it does not block the page rendering, but it will ensure that the scripts execute in order and they will execute right before DOMContentLoaded which means the rainbow callback will fire correctly.

<script defer src="rainbow/js/rainbow.js"></script>
<script defer src="rainbow/js/language/generic.js"></script>
<script defer src="rainbow/js/language/php.js"></script>

Including the scripts at the bottom of the page before the closing body without any attribute should achieve the same thing as far as I can tell.

Another option (according to the Jake Archibold article) is to inject your scripts dynamically in javascript but set the async property to false.

[
  'rainbow/js/rainbow.js',
  'rainbow/js/language/generic.js',
  'rainbow/js/language/php.js'
].forEach(function(src) {
  var script = document.createElement('script');
  script.src = src;
  script.async = false;
  document.head.appendChild(script);
});

That should not block the page render, but also guarantee that the scripts execute in order.

I'm going to close this out because I'm not sure there is a great solution for this, and it can be handled outside of rainbow.

Owner

ccampbell commented Jul 3, 2016

I'm sorry for letting this project get a bit stale. I just released version 2.0, and I would like to make things better. I was just looking at this PR and the associated issue.

While your fix may allow Rainbow.color() to be called when rainbow is loaded asynchronously it doesn't guarantee that other script that depend on Rainbow will be loaded in the right order. For example if you have

<script async src="rainbow/js/rainbow.js"></script>
<script async src="rainbow/js/language/generic.js"></script>
<script async src="rainbow/js/language/php.js"></script>

If the generic.js or javascript.js come in before rainbow.js then nothing is going to work correctly since Rainbow will be undefined and also javascript depends on having the generic patterns already loaded.

I was reading here: http://www.html5rocks.com/en/tutorials/speed/script-loading/ and here: https://webkit.org/blog/1395/running-scripts-in-webkit/ about the different kinds of script loading and it sounds like the defer attribute is actually what you should be using here.

If you change your demo page to use defer then you will get the same benefit as async where it does not block the page rendering, but it will ensure that the scripts execute in order and they will execute right before DOMContentLoaded which means the rainbow callback will fire correctly.

<script defer src="rainbow/js/rainbow.js"></script>
<script defer src="rainbow/js/language/generic.js"></script>
<script defer src="rainbow/js/language/php.js"></script>

Including the scripts at the bottom of the page before the closing body without any attribute should achieve the same thing as far as I can tell.

Another option (according to the Jake Archibold article) is to inject your scripts dynamically in javascript but set the async property to false.

[
  'rainbow/js/rainbow.js',
  'rainbow/js/language/generic.js',
  'rainbow/js/language/php.js'
].forEach(function(src) {
  var script = document.createElement('script');
  script.src = src;
  script.async = false;
  document.head.appendChild(script);
});

That should not block the page render, but also guarantee that the scripts execute in order.

I'm going to close this out because I'm not sure there is a great solution for this, and it can be handled outside of rainbow.

@ccampbell ccampbell closed this Jul 3, 2016

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