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

Uncaught error in JavaScript #2941

Closed
karmatosed opened this issue Oct 9, 2017 · 7 comments
Closed

Uncaught error in JavaScript #2941

karmatosed opened this issue Oct 9, 2017 · 7 comments
Labels
[Type] Bug An existing feature does not function as intended

Comments

@karmatosed
Copy link
Member

This was reported through the feedback form:

"Uncaught SyntaxError: missing ) after argument list on this line: ( "Promise" in window ) || document.write( "<script src="/wp-content/plugins/gutenberg/vendor/promise.97398161.js"></scr" + "ipt>" );( "fetch" in window ) || document.write( "<script src="/wp-content/plugins/gutenberg/vendor/fetch.6bd312cf.js"></scr" + "ipt>" ); Why on earth would you even use document.write? Ever heard about document.createElement('script');?"

This was from GitHub user @k1sul1.
They were using Chrome desktop.

This is an image of the issue: https://i.imgur.com/XqYKZiK.png

@karmatosed karmatosed added the [Type] Bug An existing feature does not function as intended label Oct 9, 2017
@aduth
Copy link
Member

aduth commented Oct 9, 2017

From the error message, there is a double-quote preceding the first argument of document.write where the actual output is intended to be a single quote:

'document.write( \'<script src="' .

i.e.

// Correct:
document.write( '<script src="..."></script>' );

// Incorrect:
document.write( "<script src="..."></script>" );

I see no reason why this would be output as a double-quote. Could this be a plugin replacing single quotes with double quotes?

@k1sul1
Copy link

k1sul1 commented Oct 10, 2017

Why do you want to use document.write? Only (bad) advertisers use it, and for this reason at least Chrome is blocking it entirely on bad connections.

It takes 10 minutes to write the code properly using document.createElement and event listeners.

@aduth
Copy link
Member

aduth commented Oct 10, 2017

document.write is seen as a poor practice because it is a blocking operation.

We use document.write because we need it to be a blocking operation, namely that the polyfill must be effected before any subsequent scripts load which expect those features to be in place.

The document.write is only executed when the polyfill test fails (mainly older browsers). With this in mind, the Chrome intervention is not as relevant if we assume that Chrome has the features available that we're testing.

However, we may be able to recreate the blocking behavior by injecting a new script synchronously during the polyfill test. Something like this:

http://jsbin.com/manexagufu/1/edit?html,console

Also a similar strategy advocated by polyfill.io :

https://polyfill.io/v2/docs/examples#feature-detection

Considering we may eventually want to use bleeding edge features which Chrome would not support and thus need to polyfill, this may end up being a more sustainable solution, if even not as concise.

It takes 10 minutes to write the code properly using document.createElement and event listeners.

This is needlessly hostile and unhelpful. Please have some consideration for the human who may have had reasons for the choices they made, or in the case that you take issue with the approach, offer links or references to a more appropriate solution. Better yet, it's open source, so you can submit a pull request with your suggested implementation.

@k1sul1
Copy link

k1sul1 commented Oct 11, 2017

Never meant to be hostile, just concise. The bin you linked doesn't load for me, but I don't think that you need it to be a blocking operation. Simply list your dependencies as an array, check if they exist, and simply execute a callback that calls the Gutenberg init code when all dependencies have loaded.

Crude, but working example that I wrote for this purpose a while back:

var main = function() { 
  gutenberg();
};
var dependencies = [];

if (!window.fetch) {
  dependencies.push('fetch');
}

if (!window.Promise) {
  dependencies.push('Promise');
}

function initialize(dependencies, app) {
  var dependency_count = dependencies.length;
  var dependencies_loaded = 0;
  var is_ready = function () {
    return dependency_count === dependencies_loaded;
  };
  var run_when_ready = function () {
    if (is_ready()) {
      app();
    }
  };
  var get_script = function(script) {
    var elmnt = document.createElement('script');
    elmnt.src = /path/to/scripts/polyfills/' + script + '.js';

    return elmnt;
  };

  dependencies.map(function(dependency) {
    return get_script(dependency);
  }).map(function(script) {
    script.addEventListener('load', function() {
      dependencies_loaded++;
      run_when_ready();
    });

    return document.body.appendChild(script);
  });

  run_when_ready();
}

document.addEventListener('DOMContentLoaded', function() {
  initialize(dependencies, main);
});

@karmatosed
Copy link
Member Author

I am closing this as not seen the error for a while.

@jameelmoses
Copy link

I get the same error

image

@aduth
Copy link
Member

aduth commented Nov 6, 2018

@jameelmoses Yours is a different error. Based on the contents of the message I'm inclined to think it's the same as in #10075, slated to be fixed by #11216.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants