Opened 9 years ago
Closed 9 years ago
#35873 closed defect (bug) (fixed)
wp_register_script grandchild level dependency not rendered
Reported by: | Dima Stefantsov | Owned by: | ocean90 |
---|---|---|---|
Milestone: | 4.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Script Loader | Keywords: | has-patch has-unit-tests needs-testing |
Focuses: | javascript | Cc: |
Description
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.
Attachments (4)
Change History (17)
#1
@
9 years ago
- Focuses template removed
- Keywords needs-patch has-unit-tests added
- Version trunk deleted
#2
follow-up:
↓ 3
@
9 years ago
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";
#3
in reply to:
↑ 2
@
9 years ago
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.
#4
@
9 years ago
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 old times sake, though it may not now be the best place.
#5
@
9 years ago
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?
#6
@
9 years ago
@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.
#7
@
9 years ago
@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?
#8
@
9 years ago
- Keywords needs-unit-tests added; has-unit-tests removed
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.
#9
@
9 years ago
@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.
#10
@
9 years ago
- Keywords has-patch has-unit-tests needs-testing added; needs-patch needs-unit-tests removed
- Milestone changed from Awaiting Review to 4.5
Attached 35873.4.patch which includes the tests from #25247, see also ticket:25247:11.
#11
follow-up:
↓ 12
@
9 years ago
WP_Dependencies::set_group()
was introduced in [10357]. @azaozz Any objections to the proposed change in 35873.4.patch?
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?