WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 12 days ago

#20513 closed enhancement (fixed)

Refactor $wp_scripts init checking code

Reported by: GaryJ Owned by: wonderboymusic
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.4
Component: Script Loader Keywords: has-patch needs-testing
Focuses: Cc:

Description

The functions in functions.wp-scripts.php seem to contain a lot of duplicate code in terms of checking and initializing the global $wp_scripts in each function.

Attached is rough first pass at creating a function to do this. I expect aspects of it could be done better.

I've included the first argument, to avoid relying on a global so that the function might be unit tested (pass something in, see what gets returned).

Attachments (4)

20513.diff (5.5 KB) - added by GaryJ 3 years ago.
First pass
20513.2.diff (5.8 KB) - added by scribu 3 years ago.
Second pass
20513.3.diff (5.7 KB) - added by scribu 3 years ago.
wp_scripts()
20513.4.diff (6.2 KB) - added by wonderboymusic 3 months ago.

Download all attachments as: .zip

Change History (28)

@GaryJ3 years ago

First pass

comment:1 @scribu3 years ago

+1 in general. Note that __FUNCTION__ would have to be passed along.

@scribu3 years ago

Second pass

comment:2 @scribu3 years ago

20513.2.diff:

  • accept first parameter by reference, instead of returning
  • replaced $fallback parameter with $function

Not really sure if the extra check in wp_localize_script() is worth it. Could just instantiate anyway.

Last edited 3 years ago by scribu (previous) (diff)

@scribu3 years ago

wp_scripts()

comment:3 @scribu3 years ago

20513.3.diff replaces wp_scripts_maybe_initialize() with wp_scripts(), so you can do:

wp_scripts()->do_items( $handle );

etc.

Last edited 3 years ago by scribu (previous) (diff)

comment:4 @scribu3 years ago

I don't think this init code is relevant for unit testing. If you want to unit test WP_Scripts, just create an instance manually.

Last edited 3 years ago by scribu (previous) (diff)

comment:5 @GaryJ3 years ago

That looks good to me.

Might be another ticket, but I suspect something similar can be done with $wp_styles too.

comment:6 @ocean903 years ago

  • Cc ocean90 added

comment:7 @azaozz3 years ago

Think I attempted to do something similar for 3.2 (or was it 3.3). The argument against it is that this breaks the backtrace so if a plugin author calls any of these functions too early _doing_it_wrong() would show the wrong reference.

comment:8 @scribu3 years ago

If the function reference is so important, we could pass it along:

wp_scripts( __FUNCTION__ )->do_items( $handle );

comment:10 follow-up: @GaryJ3 years ago

Related discussion: #11526.

How expensive is debug_backtrace( false )?

How many themes and plugins are still firing these functions before init action has occurred? (Nacin's relatively recent post might have reminded some of the authors to be using the wp_enqueue_scripts hook.)

Could wp_debug_backtrace_summary() be used to pinpoint the original function called?

While scribu's suggestion may well work, it seems dirty to be including an argument purely for debugging purposes.

comment:11 in reply to: ↑ 10 @SergeyBiryukov3 years ago

Replying to GaryJ:

How many themes and plugins are still firing these functions before init action has occurred?

Plugins: http://pastebin.com/szGUhsp0 (may have some false positives due to a lack of whitespace).
Themes: http://pastebin.com/bbM47tG8 (tried to filter out false positives).

comment:12 @nacin15 months ago

  • Component changed from General to Script Loader

@wonderboymusic3 months ago

comment:13 @wonderboymusic3 months ago

  • Milestone changed from Awaiting Review to Future Release

20513.4.diff makes 2 funcs, could work

Last edited 3 months ago by wonderboymusic (previous) (diff)

comment:15 @wonderboymusic3 months ago

  • Milestone changed from Future Release to 4.2

comment:16 @wonderboymusic2 months ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 31192:

functions.wp-scripts.php contains a lot of duplicated code. Make 2 new functions: wp_scripts() and wp_scripts_maybe_doing_it_wrong( $function ), to encapsulate the repeated logic.

Props GaryJ, scribu, wonderboymusic.
Fixes #20513.

comment:17 @wonderboymusic2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:18 @wonderboymusic2 months ago

In 31194:

After [31192], create a function, wp_styles(), to reduce duplicated code in functions.wp-styles.php. The style functions can reuse wp_scripts_maybe_doing_it_wrong( $function ) internally.

See #20513.

comment:19 @wonderboymusic2 months ago

In 31196:

Make _wp_scripts_maybe_doing_it_wrong( $function ) "private".

Props obenland for the thought leadership.
See #20513.

comment:20 @wonderboymusic2 months ago

In 31202:

Add @ignore to _wp_scripts_maybe_doing_it_wrong().

Props DrewAPicture for the thought leadership.
See #20513.

comment:21 @slackbot3 weeks ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:22 @DrewAPicture3 weeks ago

  • Keywords 4.2-beta added

This will ride over to beta, see [31192] [31194] [31196] [31202]. Anything left here?

comment:23 @slackbot12 days ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:24 @DrewAPicture12 days ago

  • Keywords 4.2-beta removed
  • Resolution set to fixed
  • Status changed from reopened to closed

This seems fixed. Please create new tickets for any resulting issues.

Note: See TracTickets for help on using tickets.