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

External scripts are not enqueued with necessary dependencies #18

Open
emrikol opened this issue Jun 14, 2016 · 0 comments
Open

External scripts are not enqueued with necessary dependencies #18

emrikol opened this issue Jun 14, 2016 · 0 comments
Labels

Comments

@emrikol
Copy link
Contributor

emrikol commented Jun 14, 2016

do_item() is called to let core handle external scripts, since they don't get concatenated. Unfortunately, $this->do_concat is true, so the external script tag is saved for later instead of being echo'd at the proper time for its dependents/dependencies

I've got a patch (below) but I'm not 100% sure enough to know that it wouldn't break other things. It likely needs cleaned up and tested more.

diff --git a/jsconcat.php b/jsconcat.php
index b46a4b5..ae8c61a 100644
--- a/jsconcat.php
+++ b/jsconcat.php
@@ -39,6 +39,7 @@ class WPcom_JS_Concat extends WP_Scripts {
                $handles = false === $handles ? $this->queue : (array) $handles;
                $javascripts= array();
                $siteurl = site_url();
+               $external_scripts = array();

                $this->all_deps( $handles );
                $level = 0;
@@ -73,8 +74,10 @@ class WPcom_JS_Concat extends WP_Scripts {
                                $do_concat = true;

                        // Don't try to concat externally hosted scripts
-                       if ( ( isset( $js_url['host'] ) && ( preg_replace( '/https?:\/\//', '', $siteurl ) != $js_url['host'] ) ) )
+                       if ( ( isset( $js_url['host'] ) && ( preg_replace( '/https?:\/\//', '', $siteurl ) != $js_url['host'] ) ) ) {
+                               $external_scripts[] = $handle;
                                $do_concat = false;
+                       }

                        // Concat and canonicalize the paths only for
                        // existing scripts that aren't outside ABSPATH
@@ -114,8 +117,17 @@ class WPcom_JS_Concat extends WP_Scripts {

                foreach ( $javascripts as $js_array ) {
                        if ( 'do_item' == $js_array['type'] ) {
-                               if ( $this->do_item( $js_array['handle'], $group ) )
+                               // Disable "concat" for external scripts, otherwise do_item() may print it at the wrong time
+                               if ( in_array( $js_array['handle'], $external_scripts, true ) ) {
+                                       $this->do_concat = false;
+                               }
+                               if ( $this->do_item( $js_array['handle'], $group ) ) {
                                        $this->done[] = $js_array['handle'];
+                               }
+                               // Re-enable "concat"
+                               if ( in_array( $js_array['handle'], $external_scripts, true ) ) {
+                                       $this->do_concat = true;
+                               }
                        } else if ( 'concat' == $js_array['type'] ) {
                                array_map( array( $this, 'print_extra_script' ), $js_array['handles'] );

@emrikol emrikol added the bug label Jun 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant