WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 3 days ago

#47478 closed task (blessed) (fixed)

Update Backbone.js to 1.4.0

Reported by: desrosj Owned by: adamsilverstein
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: early good-first-bug has-patch needs-dev-note commit
Focuses: Cc:

Description

Backbone.js 1.4.0 was released on February 19, 2019.

Core currently includes version 1.3.3 using the package.json file. However, the version indicated when the script is enqueued incorrectly indicates 1.2.3 is loaded.

A full list of changes since 1.3.3 can be found here: https://github.com/jashkenas/backbone/compare/1.3.3...1.4.0.

Attachments (8)

47478.diff (1.3 KB) - added by priyankkpatel 4 months ago.
Patch for wp-includes/script-loader.php
backbone.js (76.0 KB) - added by priyankkpatel 4 months ago.
backbone.js 1.4.0
backbone.min.js (24.4 KB) - added by priyankkpatel 4 months ago.
backbone.min 1.4.0
package-lock.diff (699 bytes) - added by priyankkpatel 3 months ago.
patch for file package-lock.json
47478-2.diff (2.0 KB) - added by priyankkpatel 3 months ago.
Final patch including changes in package.json, package-lock.json and wp-includes/script-loader.php
47478.3.patch (8.0 KB) - added by pierlo 6 days ago.
47478.2.diff (6.0 KB) - added by adamsilverstein 3 days ago.
47478.3.diff (4.1 KB) - added by adamsilverstein 3 days ago.

Download all attachments as: .zip

Change History (23)

@priyankkpatel
4 months ago

Patch for wp-includes/script-loader.php

#1 @priyankkpatel
4 months ago

I have attached patch for wp-includes/script-loader.php and package.json.
package.json will now download Backbone.js version 1.4.0

@priyankkpatel
4 months ago

backbone.js 1.4.0

@priyankkpatel
4 months ago

backbone.min 1.4.0

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


3 months ago

#3 follow-up: @desrosj
3 months ago

  • Keywords has-patch needs-testing needs-refresh added; needs-patch removed
  • Owner set to priyankkpatel
  • Status changed from new to assigned

Assigning to @priyankkpatel to mark good-first-bug claimed.

Thanks for the patch! The package-lock.json file will also need to be updated.

@adamsilverstein are you able to help testing this?

#4 in reply to: ↑ 3 @priyankkpatel
3 months ago

Replying to desrosj:

Assigning to @priyankkpatel to mark good-first-bug claimed.

Thanks for the patch! The package-lock.json file will also need to be updated.

@adamsilverstein are you able to help testing this?

You are welcome @desrosj

@priyankkpatel
3 months ago

patch for file package-lock.json

@priyankkpatel
3 months ago

Final patch including changes in package.json, package-lock.json and wp-includes/script-loader.php

#5 @adamsilverstein
3 months ago

Yes, I'll give this a test soon.

#6 @desrosj
4 weeks ago

I did some testing with this today. I found a few issues. They all seem to be related to this._listeners and cleaning up memory bindings. Here are scenarios that are showing the issue:

  1. Go to the Media Library and upload an image. When uploading finishes, you'll see the console error.
  2. In the Classic Editor, click Add Media, select an image, click insert. Observe the console error.
  3. In the Classic Editor, click Add Media, click Create Gallery. Observe the console error. The view also does not switch, and subsequent tab changes do not always work. Clicking Create New Gallery does nothing.

These issues also happen when working with the media modals in the block editor.

@pierlo
6 days ago

#7 @pierlo
6 days ago

Took some time out and managed to solve this. The _listeners object was being applied to the states Collection in the StateMachine (aka this.controller), instead of the StateMachine itself. This is because of a change in Backbone v1.4.0 which applies listeners to the object's public on method (in this case the StateMachine maps events and triggers to the states object).

47478.3.patch fixes this by passing the states object instead of the StateMachine. I also refactored StateMachine to remove some dead code.

Last edited 6 days ago by pierlo (previous) (diff)

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


6 days ago

#9 @adamsilverstein
4 days ago

  • Keywords needs-dev-note added

Hey @pierlo thanks for the patch and the detailed explanation about the required changes. I tested media and revisions and everything worked as expected after this change.

Since the listener changes in 1.4 might also affect plugin authors, this change deserves a devnote. https://backbonejs.org/#changelog

@desrosj this looks good to me, unless you have any final concerns I can go ahead and commit this.

#10 @desrosj
4 days ago

  • Keywords commit added; needs-testing needs-refresh removed

@adamsilverstein Let's get it in so it has time to sit until beta next week.

Thanks for working on this, @pierlo! Would you be willing to summarize the above in a short paragraph or two for a dev note? Doesn't have to be on this ticket. We can connect in Slack if you'd like.

#11 @pierlo
4 days ago

Sure thing.

#12 @adamsilverstein
3 days ago

  • Owner changed from priyankkpatel to adamsilverstein

Will do.

#13 @adamsilverstein
3 days ago

47478.2.diff limits the package-lock changes to the backbone dependency

#14 @adamsilverstein
3 days ago

47478.3.diff removes unintended changes in class-wp-user-query.php

#15 @adamsilverstein
3 days ago

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

In 46157:

External Libraries: Update Backbone.js to 1.4.0.

Upgrade Backbone to the latest stable version. Fix some issues in Media with listenTo which changed in this version, see https://backbonejs.org/#changelog.

Props desrosj, priyankkpatel, pierlo.
Fixes #47478.

Note: See TracTickets for help on using tickets.