Problem/Motivation

We do not want people to rely on Backbone as provided by Core.

Proposed resolution

Deprecate the existing library and create a new core/internal.backbone library to provide backbone for toolbar/tour/contextual/quickedit until the usages are removed.

Contrib modules that depends on core/backbone library:

acquia_commercemanager
amptheme
commerce_cart_flyout
edit_ui
gridstack
nexx_integration
obiba_mica_dashboard
panels
visualn_libraries
responsive_preview
shs
site_assistant
commerce_cart_api
devel
erd
imago
library_manager
quickedit

Issue fork drupal-3258931

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_ created an issue. See original summary.

nod_’s picture

Issue summary: View changes
nod_’s picture

Issue summary: View changes
Wim Leers’s picture

Issue tags: +JavaScript, +Drupal 10

I think this is a very reasonable approach. This discourages further adoption/new usage.

We cannot prevent code choosing to depend on the new internal asset library, but you have to go out of your way to even find this and then you need to willfully ignore it.

This does give ample warning time for maintainers of contrib/custom modules currently depending on core-provided Backbone to refactor their code.

All in all: nice trade-off 👍

Just posted 3 suggestions that would make this even more clear, then this is RTBC IMHO.

catch’s picture

Priority: Normal » Critical

+1 from me. It's a similar-ish approach we took to the core jQuery UI libraries and it gives us room to manoeuvre in Drupal 10.

Since this blocks lots of other work, and we need to deprecate the library definition in Drupal 9, bumping to critical.

nod_’s picture

Umm we should do the same for underscore too

effulgentsia’s picture

+1 to this issue. And to doing it for underscore if the contrib list relying on it is similarly small.

Can we and should we add some kind of warning (deprecation? something else?) if something other than toolbar/tour/contextual/quickedit adds backbone.internal as a dependency or tries to add it to #attached?

xjm’s picture

QuickEdit (contrib) also uses Backbone, but is missing from the IS:
https://git.drupalcode.org/project/quickedit/-/blob/1.0.x/quickedit.libr...

catch’s picture

Issue summary: View changes

Added.

xjm’s picture

xjm’s picture

Issue tags: +Drupal 9.4 target
nod_’s picture

updated MR

nod_’s picture

Issue summary: View changes

renamed the library to core/internal.backbone to put emphasis on the internal part.

nod_’s picture

xjm’s picture

Issue tags: +Needs tests

This looks pretty good. Can we add a deprecation test to ensure the old library raises the deprecation notice as intended?

nod_’s picture

Little scope creep, update the comment on the underscore library too since it got in before this one.

hooroomoo made their first commit to this issue’s fork.

hooroomoo’s picture

Status: Needs review » Reviewed & tested by the community

1. Checked that all files that had core/backbone were changed to core/internal.backbone
2. Checked that internal.backbone library was added to vendor-update.js
3. Checked that deprecation message is there and there is a test for it

xjm’s picture

Issue tags: -Needs tests

Loos good. Great work @hooroomoo!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 05d8761 and pushed to 10.0.x. Thanks!
Committed ec891b8 and pushed to 9.4.x. Thanks!

  • alexpott committed 05d8761 on 10.0.x
    Issue #3258931 by nod_, hooroomoo, xjm, catch, effulgentsia, Wim Leers:...

  • alexpott committed ec891b8 on 9.4.x
    Issue #3258931 by nod_, hooroomoo, xjm, catch, effulgentsia, Wim Leers:...
xjm’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.