krux / postscribe

Asynchronously write javascript, even with document.write.
MIT License
986 stars 157 forks source link

External Scripts, with and without document.write statements, and a Cancelled Callback #452

Closed tomgallagher closed 7 years ago

tomgallagher commented 7 years ago

Hi

Thanks for the awesome library, really useful.

I'm trying to use PostScribe to manage script injection with external scripts, some of which may have document.write calls in them, some of which may not. At the moment I'm sending all of them through Postscribe because even if there's no document.write statement it's very important to maintain the order of the external scripts.

The problem I'm having is that when I send an external script without a document.write statement in it to Postscribe, I get back a cancel object and then Postscribe ceases to function, as you might expect.

In order to work out whether there's a document.write statement in the script, I would have to find some way of downloading and searching the cross-domain scripts with some sort of regex, I guess. It seems like you're already doing something better.

What would be even better from my point of view would be a cancelled callback, which I can then work with and Postscribe would continue to perform its function for those external scripts that do have document write statements.

Tom

jnewman commented 7 years ago

Does the error callback not work for this? https://github.com/krux/postscribe/blob/master/src/postscribe.js#L65

tomgallagher commented 7 years ago

Nope doesn't get fired.

postscribe('#turboInlineScriptPlaceholder_4704_1494528816479', '<script src=http://d3pkae9owd2lcf.cloudfront.net/mb105.gz.js><\/script>', {
      done: function() {
        console.info('Dblclick script has been delivered.');
      }, 
      error: function() { console.info('Dblclick script has an error.'); }
    });

returns

Object {cancel: function}

jnewman commented 7 years ago

Are you running that on an HTTPS page? IF you give me an webpage I can test on then I'll take a look

tomgallagher commented 7 years ago

Not https no. I'm just injecting Postscribe at the moment into this page:

http://www.noobmeter.com/compareplayers/eu/all/tomg1/gallam1

And then testing the code in the console.

tomgallagher commented 7 years ago

As I said in the other thread, I'm running through a set of external scripts in an array, then adding each of those scripts in turn. I've just tested that page with an injection and then running in the console and it works fine. Could there be some issue with adding multiple Postscribe tags in quick succession, i.e. before the previous one has finished? It did all work when I was running them with thereleaseAsync: truein the options object and adding async="async"to the script tags. Shall I post some code from my project?

tomgallagher commented 7 years ago
var frag = document.createDocumentFragment();
console.log(source.substring(0, 50) + " (50 chars) now working via POSTSCRIBE with Unique ID of: " + arrayEntry[3]);
var postscribeScript = document.createElement('script');
if (arrayEntry[5]) { postscribeScript.type = arrayEntry[5]; } else { postscribeScript.type = 'text/javascript'; }
postscribeScript.charset = 'utf-8';
postscribeScript.async = false;

postscribeScript.textContent = "postscribe(\'#" + arrayEntry[3] + "\', \'<script src=" + source + "><\/script>\', { done: function() { console.info(\'Turbo Postscribe External Script (50 chars) " + source.substring(0, 50) + " has been Processed.\'); var marker = document.createElement(\'div\'); marker.setAttribute(\'data-turbo-script-type\', \'" + arrayEntry[0] + "\'); marker.setAttribute(\'data-turbo-script-content\', \'" + source.substring(0, 50) + "\'); marker.setAttribute(\'data-turbo-script-delivered\', \'" + arrayEntry[4] + "\'); (document.body||document.documentElement).appendChild(marker); }, error: function() { console.error(\'Turbo Postscribe External Script (50 chars) " + source.substring(0, 50) + " has hit an error. Starting Error Handler\'); var marker = document.createElement(\'div\'); marker.setAttribute(\'data-turbo-script-type\', '" + arrayEntry[0] + "\'); marker.setAttribute(\'data-turbo-script-content\', \'" + source.substring(0, 50) + "\'); marker.setAttribute(\'data-turbo-script-error-marker\', \'" + arrayEntry[4] + "\'); (document.body||document.documentElement).appendChild(marker); } });"
postscribeScript.setAttribute('data-optimised-script', 'true');
frag.appendChild(postscribeScript);
targetNode.appendChild(frag);
jnewman commented 7 years ago

Can you setup a webpage to debug this with?

tomgallagher commented 7 years ago

Just sent you an email. Here's the test script I was using

var array = [ "https://cdnjs.cloudflare.com/ajax/libs/jquery/3.2.1/jquery.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-compat/3.0.0-alpha1/jquery.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.pjax/1.9.6/jquery.pjax.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.12.1/jquery-ui.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-cookie/1.4.1/jquery.cookie.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.isotope/3.0.4/isotope.pkgd.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-handsontable/0.10.2/jquery.handsontable.full.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-validate/1.16.0/jquery.validate.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.transit/0.9.12/jquery.transit.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.lazyload/1.9.1/jquery.lazyload.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.gridster/0.5.6/jquery.gridster.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.imagesloaded/4.1.2/imagesloaded.pkgd.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jQuery-Knob/1.2.13/jquery.knob.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-noty/2.4.1/jquery.noty.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.colorbox/1.6.4/jquery.colorbox-min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.colorbox/1.6.4/jquery.colorbox-min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.perfect-scrollbar/0.7.0/js/perfect-scrollbar.jquery.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-infinitescroll/2.1.0/jquery.infinitescroll.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery/3.2.1/jquery.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-compat/3.0.0-alpha1/jquery.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.pjax/1.9.6/jquery.pjax.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.12.1/jquery-ui.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-cookie/1.4.1/jquery.cookie.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.isotope/3.0.4/isotope.pkgd.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-handsontable/0.10.2/jquery.handsontable.full.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-validate/1.16.0/jquery.validate.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.transit/0.9.12/jquery.transit.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.lazyload/1.9.1/jquery.lazyload.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.gridster/0.5.6/jquery.gridster.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.imagesloaded/4.1.2/imagesloaded.pkgd.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jQuery-Knob/1.2.13/jquery.knob.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-noty/2.4.1/jquery.noty.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.colorbox/1.6.4/jquery.colorbox-min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.colorbox/1.6.4/jquery.colorbox-min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery.perfect-scrollbar/0.7.0/js/perfect-scrollbar.jquery.min.js",
              "https://cdnjs.cloudflare.com/ajax/libs/jquery-infinitescroll/2.1.0/jquery.infinitescroll.min.js"];

array.forEach(function(arrayElement, index) {

    setTimeout(function(){

        var markerDiv = document.createElement('div');
        var uniqueID1 = "turboInlineScriptPlaceholder_" + Date.now();
        markerDiv.id = uniqueID1;
        var frag = document.createDocumentFragment();
        console.log(arrayElement.substring(0, 50) + " (50 chars) now working via POSTSCRIBE with Unique ID of: " + uniqueID1);
        var postscribeScript = document.createElement('script');
        postscribeScript.type = 'text/javascript';
        postscribeScript.charset = 'utf-8';
        postscribeScript.async = false;
        //deliver the postscribed entry into the page via the placeholder, with done and error handling, setting markers that are monitored by mutation summary
        postscribeScript.textContent = "postscribe(\'#" + uniqueID1 + "\', \'<script src=" + arrayElement + "><\/script>\', { done: function() { console.info(\'Turbo Postscribe External Script (50 chars) " + arrayElement.substring(0, 50) + " has been Processed.\'); var marker = document.createElement(\'div\'); marker.setAttribute(\'data-turbo-script-type\', \'" + uniqueID1 + "\'); marker.setAttribute(\'data-turbo-script-content\', \'" + arrayElement.substring(0, 50) + "\'); marker.setAttribute(\'data-turbo-script-delivered\', \'" + uniqueID1 + "\'); (document.body||document.documentElement).appendChild(marker); }, error: function() { console.error(\'Turbo Postscribe External Script (50 chars) " + arrayElement.substring(0, 50) + " has hit an error. Starting Error Handler\'); var marker = document.createElement(\'div\'); marker.setAttribute(\'data-turbo-script-type\', '" + uniqueID1 + "\'); marker.setAttribute(\'data-turbo-script-content\', \'" + arrayElement.substring(0, 50) + "\'); marker.setAttribute(\'data-turbo-script-error-marker\', \'" + uniqueID1 + "\'); (document.body||document.documentElement).appendChild(marker); } });"
        postscribeScript.setAttribute('data-optimised-script', 'true');
        frag.appendChild(postscribeScript);

        if(index % 2 === 0) {

            document.head.appendChild(markerDiv);
            markerDiv.appendChild(frag);

        } else {

            document.body.appendChild(markerDiv);
            markerDiv.appendChild(frag);

        }

    }, 2);

});
tomgallagher commented 7 years ago

Hi

OK so this was a timing issue. I was removing the first param targetnode from the page, after Postscribe checks for its presence but before the script was actually written to the page. So Postscribe just failed silently. Maybe another check for the target node before writing to the page? And maybe I should leave tidying up until I've actually got the whole thing working! Anyway, thanks for your time @jnewman

Tom

jnewman commented 7 years ago

I'll add a check in a future release