Make WordPress Core

Opened 16 months ago

Last modified 3 months ago

#61755 new defect (bug)

Use WPINC constant in script-loader.php

Reported by: kkmuffme's profile kkmuffme Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: close
Focuses: Cc:

Description

https://github.com/WordPress/wordpress-develop/blame/trunk/src/wp-includes/script-loader.php#L135 and various others in this file

These should be changed to use WPINC to keep it consistent

Change History (1)

#1 @jonsurrell
3 months ago

  • Keywords close added

It does seem that WPINC is always defined to be wp-includes. It seems like the convention is to use WPINC for file paths but not URLs. That leads to patterns like this:

<?php
foreach ( $development_scripts as $script_name ) {
        $assets = include ABSPATH . WPINC . '/assets/script-loader-' . $script_name . '.php';
        if ( ! is_array( $assets ) ) {
                return;
        }
        $scripts->add(
                'wp-' . $script_name,
                '/wp-includes/js/dist/development/' . $script_name . '.js',
                $assets['dependencies'],
                $assets['version']
        );
}

I think you're suggesting it should be used for URLs as well:

'/wp-includes/js/dist/development/' . $script_name . '.js'

becomes

'/' . WPINC . '/js/dist/development/' . $script_name . '.js'

Is that what you recommend? Would that provide any benefits other than consistency?

I see that the usage does have some consistency, where the full path is written our for asset URLs. The form that uses concatenation with WPINC seems to trade the risk of typing /wp-includes correctly for the risk of concatenating WPINC incorrectly. There's also the inherent risk that a change has vs. the existing implementation that has seen real world usage.

Note: See TracTickets for help on using tickets.