Make WordPress Core

Opened 13 months ago

Last modified 5 months ago

#45785 reopened enhancement

Update to Underscore 1.9.1

Reported by: mukesh27 Owned by: desrosj
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: has-patch early needs-testing
Focuses: Cc:
PR Number:


Let's keep WordPress up-to-date with the latest Underscore version.

WordPress 5.0.2 includes Underscore 1.8.3 as of this report but Underscore 1.9.1 is the latest version.

See Underscore changelog at https://underscorejs.org/#changelog

Attachments (2)

45785.diff (149.8 KB) - added by Hareesh Pillai 8 months ago.
Patch for upgrading underscore to v 1.91.
45785.2.diff (1001 bytes) - added by Hareesh Pillai 5 months ago.
Changes to package.json and package-lock.json file

Download all attachments as: .zip

Change History (11)

@Hareesh Pillai
8 months ago

Patch for upgrading underscore to v 1.91.

#1 @Hareesh Pillai
8 months ago

  • Keywords has-patch added

#2 @Hareesh Pillai
8 months ago

  • Keywords early added

#3 @desrosj
8 months ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 5.3

Thanks for the patch, @hareesh-pillai!

Underscore is now managed by the package.json file. The version needs to be updated in that file and the appropriate changes to the package-lock.json file also need to be included in the patch.

@Hareesh Pillai
5 months ago

Changes to package.json and package-lock.json file

#4 @desrosj
5 months ago

  • Keywords commit added; needs-refresh removed

Thanks for your work on this, @hareesh-pillai!

I did some testing and discovered a console error. But it turns out this already exists in core. I opened #48021 for that.

#5 @desrosj
5 months ago

  • Owner set to desrosj
  • Status changed from new to assigned

#6 @desrosj
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 46092:

External Libraries: Update Underscore to 1.9.1.

Changes: https://github.com/jashkenas/underscore/compare/1.8.3...1.9.1

Props mukesh27, hareesh-pillai.
Fixes #45785

#7 @SergeyBiryukov
5 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to address test failures:

>> tinymce.obsolete - HTML elements non-conforming to HTML 5.0
>> Message: TypeError: undefined is not an object (evaluating 'a.replace')
>> Actual: null
>> Expected: undefined
>> file:///home/travis/build/WordPress/wordpress-develop/build/wp-includes/js/underscore.min.js:1

>> tinymce.obsolete - HTML elements non-conforming to HTML 5.0
>> Message: Expected 6 assertions, but 7 were run
>> Actual: null
>> Expected: undefined
>> test@file:///home/travis/build/WordPress/wordpress-develop/tests/qunit/vendor/sinon-qunit.js:61:21

The failures come from tests/qunit/wp-includes/js/tinymce/tinymce-obsolete.js, however renaming that file just leads to similar failures in other files.

It's also worth noting that the failures are inconsistent, sometimes I get a different message:

>> Video Media Widget - video widget control renderPreview                                                                                                  
>> Message: TypeError: undefined is not an object (evaluating 'a.replace')                                                                                  
>> Actual: null                                                                                                                                             
>> Expected: undefined                                                                                                                                      
>> file:///S:/home/wordpress.test/develop.svn.wordpress.org/build/wp-includes/js/underscore.min.js:1   

I was able to track down the issue to this commit cleaning up the _.debounce() function, before that commit the tests pass successfully.

Not sure what's specifically causing the issue here, I guess there's some kind of a race condition between Underscore's _.debounce() and QUnit's internal timeout handling.

#8 @SergeyBiryukov
5 months ago

In 46093:

External Libraries: Revert [46092] pending test failure investigation.

See #45785.

#9 @desrosj
5 months ago

  • Keywords needs-testing added; commit removed
  • Milestone changed from 5.3 to Future Release

I looked at this a bit today. The _.debounce() function invokes _.delay() with 4 arguments, but that function appears to only accept 3 arguments. Removing this from this line in Underscore fixes the problem. It's strange that only one are two tests are failing (and with inconsistent messages), but all of the front end functionality seems to be working as best I can tell.

Going to move this to Future Release for now since this is not a pressing change. If someone a little more familiar with Underscore is able to investigate in time for 5.3, we can move it back. But, it's possible that an upstream PR may be needed.

Note: See TracTickets for help on using tickets.