WP_Dependencies' constructor should accept a string/array value for $deps rather than converting a string to an empty array

[vhauri]

Currently, WP_Dependencies' constructor takes any $deps value that's not an array and converts it to an empty array. I propose bringing the constructor's behavior more in line with standard WP function paramater behavior by accepting a string as a single dependency. Essentially this involves adding an is_string() check on $deps before checking if it's not an array, and creating an array with a single element in the case that it is a string.

if ( !is_array($this->deps) )
  $this->deps = array();

becomes

if ( is_string( $this->deps ) )
  $this->deps = (array) $this->deps;
elseif ( !is_array( $this->deps ) )
  $this->deps = array();

See the attached diff for a proposed patch.

[vhauri]

Patch to allow a single $deps in WP_Dependencies to be passed as a string

[scribu]

+1, but I think array( $this->deps ) is clearer than (array) $this->deps.

[vhauri]

Updated diff with clearer array typecasting as suggested by @scribu

[misterbisson]

+1

[markjaquith]

Looks good. Nice common sense API improvement.

[markjaquith]

In [21420]:

Let WP_Dependencies accept a string for a single dependency instead of requiring an array wrapper. props vhauri. fixes #20683

[SergeyBiryukov]

Follow-up: #21467

[dd32]

In [21421]:

Fix the plupload script enqueue, props SergeyBiryukov, Fixes #21467. See #20683.

[azaozz]

In [21481]:

When WP_Dependencies accept a string for a single dependency, make sure the string is not empty, see #20683

[nacin]

This seemed like an easy win, but will likely be this release's hey-look-we-broke-your-scripts bug if it gets into 3.5 final. See #22111. Re-opening.

[scribu]

Ugh... yes, let's revert it.

[DrewAPicture]

From @nacin's ​comment on #22111, seems like it's not currently worth rocking the boat. Is it coming out or staying in?

[ryan]

Also: #21520

[ryan]

Revert

[ryan]

In [22286]:

Revert [21420] and [21481]. Accepting a string caused back compat problems including the possibility of revealing previously hidden circular dependencies resulting in infinite loops.

fixes #20683 #22111
see #21520

[Otto42]

I don't accept that this is unsolvable.

The problem here seems to be that by allowing $deps to be a string, then somebody who had accidentally put a version there (like in #22111) would get a non-existent dependency. The code in WP_Dependency would then simply not include the code because of a missing dependency.

This problem can be solved by simply ignoring non-existent dependencies and letting the script/stylesheet load anyway. Yes, it may be broken, but it will definitely be broken if it doesn't load at all.

Dependencies define the order of loading, but we don't necessarily have to state that they will also define absolute dependence upon the defined enqueued items. If somebody has a non-existent dependency, then that's clearly a coding error, and throwing a _doing_it_wrong but loading the scripts anyway would make more sense.

Looks to me like you can just remove those checks in function all_deps, or change them to keep going anyway but throw a debugging error.

[rmccue]

Replying to Otto42:

I don't accept that this is unsolvable.

The problem here seems to be that by allowing $deps to be a string, then somebody who had accidentally put a version there (like in #22111) would get a non-existent dependency. The code in WP_Dependency would then simply not include the code because of a missing dependency.

This problem can be solved by simply ignoring non-existent dependencies and letting the script/stylesheet load anyway. Yes, it may be broken, but it will definitely be broken if it doesn't load at all.

Dependencies define the order of loading, but we don't necessarily have to state that they will also define absolute dependence upon the defined enqueued items. If somebody has a non-existent dependency, then that's clearly a coding error, and throwing a _doing_it_wrong but loading the scripts anyway would make more sense.

Surprisingly, I agree with you. Dependencies do mean that the code relies on it, but getting a Javascript error with jQuery is undefined (e.g.) seems a little easier to debug than the script not loading at all.

[ocean90]

#35841 was marked as a duplicate.

[desrosj]

The latest patch needs a refresh against current trunk.

I'm also considering just closing this out as wontfix. WP_Depenencies was introduced in 2.6, and specifying dependencies in array format when using wp_enqueue_script()/wp_enqueue_style() is now such a common practice that I don't know that the benefit of this change still exists.

Marking close/2nd-opinion for others to weigh in.

[JeffPaul]

Slow concur here with @desrosj and accepting the close on this as the value to change is outweighed by further effort here to do so.