Problem/Motivation

In order to remove backbone form contextual we need to deprecate and mark the public API of contextual links as internal.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

The JavaScript API for Contextual module has been marked internal, and various parts of this will be refactored and/or removed in Drupal 10. Contributed modules should interact with contextual module only via the PHP API.

CommentFileSizeAuthor
#12 3277311-12-d10.patch8.53 KBlauriii

Issue fork drupal-3277311

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

Status: Active » Needs review
Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

I can't find a single detail wrong with this: neither with the MR nor with the draft change record.

I had to look up the docs for Drupal.deprecatedProperty, because there is only one use of it in HEAD so far (user.es6.js).

Having read the change record that introduced it (https://www.drupal.org/node/3082634), this is AFAICT correct 👍

lauriii’s picture

Status: Reviewed & tested by the community » Needs review

Is there a particular reason we are not triggering warnings for the deprecated properties in Drupal.contextualToolbar?

From release management perspective, should we say that the API will be private/internal from 10.0.x instead of saying that it will be removed? This would also make it clear what is needed when these comments are being removed.

nod_’s picture

My thinking that since we say Drupal.contextualToolbar is internal and we couldn't find any code in contrib that extends 'model' (and all the backbone stuff for contextual), it'd be fine like this.

nod_’s picture

about removal, I don't mind either way. starting with 10.x nobody should rely on it. Saying we remove them would allow us to clean up everything at once instead of doing it over 10.x and 11.x

The fact that we couldn't find any contrib module extending any of this I'm thinking we can make our lives simpler.

Wim Leers’s picture

I agree with #6.

The intent for 100% of the Backbone-based JS functionality was always to make it possible for contrib/custom code to listen to model changes — that was the only API.

larowlan’s picture

Similar thoughts, the api was the event, so not fussed either way

catch’s picture

Status: Needs review » Reviewed & tested by the community

I think #6 is reasonable too, always a bit borderline for things that should have been internal from the beginning that we also want to remove without any replacement. Going to move back to RTBC since I think @lauriii's feedback has been answered OK.

catch’s picture

Issue summary: View changes
Issue tags: +9.4.0 release notes

Trying to add a release note.

lauriii’s picture

Here are the changes from MR rerolled for D10.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: 3277311-12-d10.patch, failed testing. View results

  • lauriii committed 68f22f8 on 10.0.x
    Issue #3277311 by nod_, Wim Leers, catch, larowlan: Deprecate and mark...

  • lauriii committed 7942ea7 on 9.5.x
    Issue #3277311 by nod_, Wim Leers, catch, larowlan: Deprecate and mark...

  • lauriii committed bc0c266 on 9.4.x
    Issue #3277311 by nod_, Wim Leers, catch, larowlan: Deprecate and mark...
lauriii’s picture

Status: Needs work » Fixed

Committed 68f22f8 and pushed to 10.0.x. Also committed the MR to 9.5.x and cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

Published the change record.

xjm’s picture

Status: Closed (fixed) » Needs work
Issue tags: +Needs change record updates

The CR here is a little thin -- maybe we could provide a list of the deprecations in it?

nod_’s picture

added the list of deprecated JS objects to the change notice

longwave’s picture

Status: Needs work » Fixed
Issue tags: -Needs change record updates

Change record looks OK to me now.

Status: Fixed » Closed (fixed)

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