wp_register_script grandchild level dependency not rendered

[Dima Stefantsov]

​http://wordpress.stackexchange.com/questions/218230/wp-enqueue-script-with-dependencies-doesnt-work

I'm having this code:

wp_register_script('parent','parent.js', array('child'), '1', true);
wp_register_script('child', 'child.js', array('grandchild'), '1', true);
wp_register_script('grandchild', 'grandchild.js', array(), '1', true);
wp_enqueue_script('parent');

and it works fine, rendering grandchild.js, then child.js, then parent.js in footer.

Every combination of just parent and child works fine, regardless of TRUE or FALSE 'render in footer'. But when I register 'child' to be rendered in head instead:

wp_register_script('parent','parent.js', array('child'), '1', true);
wp_register_script('child', 'child.js', array('grandchild'), '1', FALSE);
wp_register_script('grandchild', 'grandchild.js', array(), '1', true);
wp_enqueue_script('parent');

Then child.js gets rendered in head, parent.js gets rendered in footer, and grandchild.js is not rendered at all ''' Even though there is obviously a dependency for it.

Looks like a bug to me.

On a first glance, it looked like a https://core.trac.wordpress.org/ticket/35643 , but I have just downloaded night wp build ​https://wordpress.org/nightly-builds/wordpress-latest.zip and problem still persists.

[ocean90]

Hello Dima, welcome to Trac!

Thanks for your report. What do you think should be the expected output? Load all scripts in header like 35873.patch​?

[Dima Stefantsov]

Hi Dominik. Thanks!

I expect same behaviour as when two scripts queued: wordpress tries to render everything where it was registerd to be, moving footer children to header if parent requires header.

$expected_head  = "<script type='text/javascript' src='http://example.org/grandchild.js?ver=1'></script>\n"; 
$expected_head .= "<script type='text/javascript' src='http://example.org/child.js?ver=1'></script>\n"; 

$expected_footer = "<script type='text/javascript' src='http://example.org/parent.js?ver=1'></script>\n"; 

[ocean90]

Replying to Dima Stefantsov:

I expect same behaviour as when two scripts queued: wordpress tries to render everything where it was registerd to be, moving footer children to header if parent requires header.

Makes sense, updated test in 35873.2.patch​.

[gitlost]

See #25247 (which has been bypassed a bit by #35643 and changeset [36550]) where in comment https://core.trac.wordpress.org/ticket/25247#comment:11 it was suggested forcing any dependencies to the lowest common group would fix this issue. I'll upload updated patches there, for all times sake, though it may not now be the best place.

[Dima Stefantsov]

Wow, so this is going on for 2 years already!
Having trust that wp_enqueue_script will not silently forget about some of scripts seems pretty important to me. How soon do you think it will be fixed?

[ocean90]

@dima-stefantsov Can give 35873.3.patch​ a try please? Found it in a patch by @gitlost, see ticket:25247:25247_dependencies.diff​.

@gitlost FYI: Your latest patch on #25247 seems not to fix this issue.

[Dima Stefantsov]

@ocean90 as far as I can see, it works as I expect it after I wrote

$this->group = $group = min($this->group, $group);

from patch 35873.3.patch

Everything works now. Will it appear in next minor WP release, if you confirm it doesn't break any tests?

[ocean90]

Will it appear in next minor WP release, if you confirm it doesn't break any tests?

Minor releases are only to fix regressions. It could land in 4.5 but therefore it needs a few more unit tests since there are currently none around group handling.

[gitlost]

@gitlost FYI: Your latest patch on #25247 seems not to fix this issue.

Oh, it seems functionally equivalent to 35873.3.patch, is there a particular test it fails?

Minor releases are only to fix regressions. It could land in 4.5 but therefore it needs a few more unit tests since there are currently none around group handling.

There's more group handling tests in 25247_tests6.diff.

[ocean90]

Includes tests by @gitlost

[ocean90]

Attached 35873.4.patch​ which includes the tests from #25247, see also ticket:25247:11.

[ocean90]

WP_Dependencies::set_group() was introduced in [10357]. @azaozz Any objections to the proposed change in 35873.4.patch​?

[azaozz]

Replying to ocean90:

35873.4.patch​ looks good. That code is meant to reduce the "group number" when there is a recursion and the dependency has higher "group number", so dependencies are always printed before the main script.

[ocean90]

In 36604:

Script Loader: Fix missing script output when the groups of dependencies are different.

Aka: Don't lose the grandchild.

Props gitlost, ocean90.
Fixes #35873.