[wp-trac] [WordPress Trac] #35956: Script's dependencies are always moved to header

[WordPress Trac]

#35956: Script's dependencies are always moved to header
------------------------------+------------------------
 Reporter:  stephenharris     |       Owner:  ocean90
     Type:  defect (bug)      |      Status:  reviewing
 Priority:  normal            |   Milestone:  4.5
Component:  Script Loader     |     Version:  trunk
 Severity:  normal            |  Resolution:
 Keywords:  needs-unit-tests  |     Focuses:
------------------------------+------------------------

Comment (by stephenharris):

 Replying to [comment:3 gitlost]:
 > It's very confusing but the `$group` isn't always `false` for
 `WP_Dependencies::set_group()`, as it's set in `WP_Scripts::set_group()`.

 Ah yes, of course. So I think the issue really was with [36604].

 Your sorting patch does appear to be work, but I think I've got to the
 root of the problem which addresses this and the issue reported in
 [#35873].

 When processing dependencies `$this->group` will be the minimum of the
 script's registered group and all preceding '''siblings'''. This is wrong
 because only a scripts 'ancestors' in the dependency chain should affect
 where it is loaded. Effectively `$this->group` introduced a form of global
 state which potentially corrupted the group of dependencies.  Sorting
 covers up this problem.

 The real issue in [#35873] was that script were  not moving their
 dependencies to a lower group when necessary.  E.g. if you reversed patch
 [36604], I found the following failed to load the child script:

 {{{
 wp_register_script( 'child-footer', '/child-footer.js', array(), null,
 true );
 wp_register_script( 'parent-header', '/parent-header.js', array( 'child-
 footer' ), null, false );
 wp_enqueue_script( 'parent-header' );
 }}}

 The 'grandfather' part of that ticket was irrelevant.

 There are two reasons why dependencies aren't being appropriately moved.
 First of all in this line:
 https://github.com/WordPress/WordPress/blob/484d9ec6a6763690b4060a90af05cc107c941b64
 /wp-includes/class.wp-dependencies.php#L166 we pass `$group`: the purpose
 here is to ensure that the dependencies are loaded in that group or lower,
 but we are passing the wrong value. Our parent script could have moved
 with `WP_Scripts::set_group`.

 Secondly `WP_Dependencies::all_deps()` ''always'' has `$group` evaluated
 to false because it is never given a third parameter  by
 `WP_Scripts::all_deps()` (see
 https://github.com/WordPress/WordPress/blob/484d9ec6a6763690b4060a90af05cc107c941b64
 /wp-includes/class.wp-scripts.php#L359).

 Patch to follow...

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35956#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform