Make WordPress Core

Opened 7 months ago

Closed 7 months ago

Last modified 7 months ago

#60048 closed enhancement (fixed)

Fix the declared Lodash version

Reported by: jadpm's profile jadpm Owned by: jorbin's profile jorbin
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: has-patch has-unit-tests
Focuses: javascript Cc:

Description

The Lodash version stated in package.json and loaded by Core is 4.17.21. However, we declare it as 4.17.19 in script-loader.php.

Change History (9)

This ticket was mentioned in PR #5755 on WordPress/wordpress-develop by decodekult.


7 months ago
#1

  • Keywords has-patch added

Sync the declared version number with the one that is loaded. It seems that in the last library update, the declared version was not updated to match it.

Trac ticket: https://core.trac.wordpress.org/ticket/60048

#2 @swissspidy
7 months ago

  • Milestone changed from Awaiting Review to 6.5

Good catch! And thanks for the PR.

It would be super cool if we could add a PHPUnit test that catches this in the future:

  1. Call wp_default_packages_vendor
  2. Loop through the registered vendor scripts
  3. Compare their version with what's in package.json
  4. Fail if there is a mismatch

This ticket was mentioned in PR #5757 on WordPress/wordpress-develop by @jorbin.


7 months ago
#3

  • Keywords has-unit-tests added

See: https://core.trac.wordpress.org/ticket/60048

Based on @swissspidy's suggestion, I added a simple test to ensure the registered version is the same as the version loaded via package.json.

The test only includes a subset of scripts since the others are renamed. Rather than adjusting the way scripts are loaded, this makes the list of which scripts to test a hard-coded version.

This incorporates the fix from #5755.

#4 @jorbin
7 months ago

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

In 57185:

External Libraries: For Lodash, sync the declared version number with the one that is loaded.

In [50941] the version of lodash was updated, however the version inside wp_default_packages_vendor was not updated at the same time. This updates the version to correctly reflect the version that is loaded.

Also adds some basic tests for the scripts in wp_default_packages_vendor that match the name of the package from package.json to help prevent errors like this in the future.

Props jadpm, jorbin, swissspidy.
Fixes #60048. See #52991.

#7 @TobiasBg
7 months ago

@jorbin: The PHP function name test_wp_default_packages_vendor_lodash should probably not have the _lodash, to not create confusion (as the test actually covers multiple libraries)?

#8 @jorbin
7 months ago

In 57187:

Build/Test: Update name of test to show its breadth.

The test covers multiple libraries, not just lodash.

Follow up to [57185].

Props TobiasBg.
Fixes #60048.

#9 @skithund
7 months ago

@jorbin $provider = array(); in _scripts_from_package_json() might be unnecessary?

Note: See TracTickets for help on using tickets.