WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 5 months ago

#23816 assigned enhancement

wp_enqueue_script|style() dependency failures are silent

Reported by: johnbillion Owned by: rmccue
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: needs-docs needs-patch
Focuses: Cc:

Description (last modified by markjaquith)

If I call wp_enqueue_script() or wp_enqueue_style() with a dependency that doesn't exist then I don't get told about it.

Example:

wp_enqueue_script(
	'my-script',
	get_stylesheet_directory_uri() . '/my-script.js',
	array( 'script-that-does-not-exist' )
);

Actual result: nothing is enqueued, and no notices are thrown.

Expected result: When WP_DEBUG is true, I should get a PHP notice warning me that the dependency script doesn't exist.

Attachments (3)

23816.diff (1.5 KB) - added by rmccue 6 months ago.
Fire an action if dependencies fail
23816.2.diff (3.5 KB) - added by rmccue 6 months ago.
Add two filters (one per dependency type)
23816.3.diff (4.0 KB) - added by rmccue 5 months ago.
Trigger errors on development by default

Download all attachments as: .zip

Change History (12)

comment:1 follow-up: @SergeyBiryukov2 years ago

Actual result: 'my-script' gets enqueued

Could not reproduce that. My steps:

  1. Created an empty js/my-script.js file in Twenty Thirteen child theme.
  2. Added this to child theme's functions.php:
    function twentythirteen_my_scripts() {
    	wp_enqueue_script(
    		'my-script',
    		get_stylesheet_directory_uri() . '/js/my-script.js',
    		array( 'script-that-does-not-exist' )
    	);
    }
    add_action( 'wp_enqueue_scripts', 'twentythirteen_my_scripts' );
    
  3. The script was not enqueued.
  4. Once I replaced 'script-that-does-not-exist' with 'jquery', the script was enqueued.

comment:2 in reply to: ↑ 1 @johnbillion2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Replying to SergeyBiryukov:

Could not reproduce that. The script was not enqueued.

Confirmed. Enqueing a file with a missing dependency does not enqueue the file. I'm not sure what I was doing.

comment:3 @markjaquith11 months ago

  • Component changed from General to Script Loader
  • Description modified (diff)
  • Milestone set to Awaiting Review
  • Priority changed from low to normal
  • Resolution invalid deleted
  • Status changed from closed to reopened

Opening this back up. The original report is mostly valid. If you register/enqueue something with a bad dependency, you get silent failure. Nothing is output, and no notices are thrown. I typo'd 'wp-utils' instead of 'wp-util' and it was maddening trying to figure out why no scripts were printing.

@rmccue6 months ago

Fire an action if dependencies fail

comment:4 @rmccue6 months ago

Constantly running into this. Added a patch that fires an action here, allowing tooling to report a broken dependency chain, but this doesn't report anything in core. We might want to move this into the subclasses instead and simply call it from the parent to allow more specific hooks, plus differentiation between scripts and styles.

(Note: this file is technically part of BackPress, so I'm not sure if we should be doing something with actions. There aren't any in the file currently. That said, BackPress hasn't been synched in a long time, so I don't think it's an issue.)

We could also add a default handler into core that outputs these as HTML comments if you have WP_DEBUG on and we're doing_action( 'wp_head' )

@rmccue6 months ago

Add two filters (one per dependency type)

comment:5 @rmccue6 months ago

  • Keywords has-patch added

Updated patch; now adds wp_scripts_missing_dependency and wp_styles_missing_dependency as separate filters.

comment:6 follow-up: @johnbillion5 months ago

  • Keywords needs-docs added

We might as well go the whole hog and add a default hook onto those actions which triggers a PHP notice when WP_DEBUG is on.

The third parameter needs adding to the hook docs too.

@rmccue5 months ago

Trigger errors on development by default

comment:7 in reply to: ↑ 6 @rmccue5 months ago

Replying to johnbillion:

We might as well go the whole hog and add a default hook onto those actions which triggers a PHP notice when WP_DEBUG is on.

The third parameter needs adding to the hook docs too.

Fixed on both by doing it inline, ala _doing_it_wrong/_deprecated_{file,function}. Can switch to a default filter if you'd prefer, but it's not really reusable code since it has different messages for each.

comment:8 @rmccue5 months ago

  • Keywords needs-patch added; has-patch removed

Ran into an issue with this one. Dependency resolution takes place for all scripts in the header, and again in the footer. If you register a script to output in the footer, but it depends on something registered after wp_head, you get an error despite the script being output correctly.

We need to take this into account when passing in $handles to all_deps; an array_filter should suffice.

comment:9 @rmccue5 months ago

  • Owner set to rmccue
  • Status changed from reopened to assigned
Note: See TracTickets for help on using tickets.