Automattic / Automattic/nginx-http-concat
External scripts are not enqueued with necessary dependencies
- Dominant language
- PHP
- Stars
- 120
- Forks
- 37
- PR merge metrics
- No merged PRs in 30d
Description
[`do_item()`](https://github.com/Automattic/nginx-http-concat/blob/master/jsconcat.php#L117) 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](https://github.com/WordPress/WordPress/blob/4.5.2/wp-includes/class.wp-scripts.php#L358)
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'] );
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading do_item() in jsconcat.php alongside the referenced WordPress class.wp-scripts.php implementation. Trace how external scripts and their dependencies are processed, then verify that external tags are emitted in dependency order while concatenated scripts still behave as before.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, php, wordpress
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100