Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#47478 closed task (blessed) (fixed)

Update Backbone.js to 1.4.0

Reported by: desrosj's profile desrosj Owned by: adamsilverstein's profile adamsilverstein
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: early good-first-bug has-patch has-dev-note
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 6 years ago.
Patch for wp-includes/script-loader.php
backbone.js (76.0 KB) - added by priyankkpatel 6 years ago.
backbone.js 1.4.0
backbone.min.js (24.4 KB) - added by priyankkpatel 6 years ago.
backbone.min 1.4.0
package-lock.diff (699 bytes) - added by priyankkpatel 6 years ago.
patch for file package-lock.json
47478-2.diff (2.0 KB) - added by priyankkpatel 6 years 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 years ago.
47478.2.diff (6.0 KB) - added by adamsilverstein 6 years ago.
47478.3.diff (4.1 KB) - added by adamsilverstein 6 years ago.

Download all attachments as: .zip

Change History (28)

@priyankkpatel
6 years ago

Patch for wp-includes/script-loader.php

#1 @priyankkpatel
6 years 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
6 years ago

backbone.js 1.4.0

@priyankkpatel
6 years ago

backbone.min 1.4.0

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


6 years ago

#3 follow-up: @desrosj
6 years 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
6 years 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
6 years ago

patch for file package-lock.json

@priyankkpatel
6 years ago

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

#5 @adamsilverstein
6 years ago

Yes, I'll give this a test soon.

#6 @desrosj
6 years 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 years ago

#7 @pierlo
6 years 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 years ago by pierlo (previous) (diff)

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


6 years ago

#9 @adamsilverstein
6 years 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
6 years 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
6 years ago

Sure thing.

#12 @adamsilverstein
6 years ago

  • Owner changed from priyankkpatel to adamsilverstein

Will do.

#13 @adamsilverstein
6 years ago

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

#14 @adamsilverstein
6 years ago

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

#15 @adamsilverstein
6 years 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.

This ticket was mentioned in Slack in #core-js by adamsilverstein. View the logs.


6 years ago

#17 @joyously
6 years ago

Was the Customizer tested with the new version?

#18 @adamsilverstein
6 years ago

Great question @joyously - I did some basic testing in the Customizer: additional testing would be welcome!

In general, I don't think the bigger changes would affect the Customizer.

#19 @desrosj
6 years ago

  • Keywords has-dev-note added; needs-dev-note commit removed

#20 @adamsilverstein
6 years ago

@joyously Wanted to note that I went back and spent some time testing the customizer with several bundled themes and didn't note any issues. Now that the code is in our released betas, we can expect much wider testing and hopefully we can catch any possible regressions.

Note: See TracTickets for help on using tickets.