Problem/Motivation

common.inc has a drupal_js_defaults() function that has not been used in core since 2015.

It is also unused in contrib: http://grep.xnddx.ru/search?text=drupal_js_defaults&filename=

Proposed resolution

Deprecate it for removal in Drupal 10. Reword the two docblocks that still refer to it.

Remaining tasks

None

User interface changes

None

API changes

The function drupal_js_defaults() has been deprecated.

Data model changes

None

Release notes snippet

TBD

CommentFileSizeAuthor
#14 interdiff_9_12.txt994 bytesbeatrizrodrigues
#14 3197553-12.patch3.39 KBbeatrizrodrigues
#9 3197553-9.patch3.39 KBbeatrizrodrigues
#4 3197553-4.patch3.4 KBlongwave
#3 3197553.patch3.35 KBlongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Parent issue: » #3015538: [META] Deprecate contents of common.inc
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Active » Needs review
FileSize
3197553.patch3.35 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
FileSize
3197553-4.patch3.4 KB

Change record: https://www.drupal.org/node/3197679

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work 
+++ b/core/includes/common.inc
@@ -200,9 +200,13 @@ function base_path() {
+ * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. No direct
...
+  @trigger_error('drupal_js_defaults() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. No direct replacement is provided. See https://www.drupal.org/node/3197679', E_USER_DEPRECATED);

Deprecate in D9.4 or D10 and remove in D11.

The rest of the patch is RTBC for me.

beatrizrodrigues’s picture

beatrizrodrigues
She/Her
CreditAttribution: beatrizrodrigues commented
Assigned: Unassigned » beatrizrodrigues
beatrizrodrigues’s picture

beatrizrodrigues
She/Her
CreditAttribution: beatrizrodrigues commented
Assigned: beatrizrodrigues » Unassigned
Status: Needs work » Needs review
FileSize
3197553-9.patch3.39 KB

I did the reroll of the patch, and I also applied the changing about the deprecation message that was suggested at #7. (Changed 9.2.0 to 9.4.0 and 10.0.0 to 11.0.0).

Status: Needs review » Needs work

The last submitted patch, 9: 3197553-9.patch, failed testing. View results

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented

Deprecate in D9.4 or D10 and remove in D11.

(emphasis mine)

@daffie: Why deprecated in D11 instead of D10, or would that only be the case if we deprecate in D10 instead of D9.4?

daffie’s picture

daffie CreditAttribution: daffie commented

Why deprecated in D11 instead of D10, or would that only be the case if we deprecate in D10 instead of D9.4?

We need to give site owners time to remove/change the use of the function drupal_js_defaults(). Say you have a Drupal site that uses that function and you want to migrate from D9.3 to D10. Your D9.3 gives you no deprecation warning message and in D10 the function is removed. That to me is not very site owner/developer friendly. Lets give contrib and custom code some time to make the change.

daffie’s picture

daffie CreditAttribution: daffie commented 
+++ b/core/includes/common.inc
@@ -201,9 +201,13 @@ function base_path() {
+ * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. No direct
...
+  @trigger_error('drupal_js_defaults() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. No direct replacement is provided. See https://www.drupal.org/node/3197679', E_USER_DEPRECATED);

In these lines need the deprecation message also be updated.

beatrizrodrigues’s picture

beatrizrodrigues
She/Her
CreditAttribution: beatrizrodrigues commented
FileSize
3197553-12.patch3.39 KB
interdiff_9_12.txt994 bytes

I missed a part where the deprecation message appears.

beatrizrodrigues’s picture

beatrizrodrigues
She/Her
CreditAttribution: beatrizrodrigues commented
Status: Needs work » Needs review
daffie’s picture

daffie CreditAttribution: daffie commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

All code changes look good to me.
The IS and the CR are in order.
For me it is RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed a2c58dda71d to 10.0.x and fbcea5f26b7 to 9.4.x. Thanks!

  • alexpott committed a2c58dd on 10.0.x 
    Issue #3197553 by beatrizrodrigues, longwave, daffie: Deprecate...

  • alexpott committed fbcea5f on 9.4.x 
    Issue #3197553 by beatrizrodrigues, longwave, daffie: Deprecate...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

I'm not sure that deprecating this for Drupal 11 is really necessary - since there is no replacement. But we can change that in a follow-up.

daffie’s picture

daffie CreditAttribution: daffie commented

I'm not sure that deprecating this for Drupal 11 is really necessary

Somebody might be using the function in some custom code. Maybe by mistake.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

Discussed this with the release managers (@xjm and @catch) and we agreed to deprecate this for Drupal 10. It's not that often used and if it is there is no replacement - the solution is to not use it so you don't need to be on 9.4 to fix this.

Can someone open a follow-up to change the deprecation to D10 and to remove this function on 10.0.x? Thanks.

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented

Can someone open a follow-up to change the deprecation to D10 and to remove this function on 10.0.x? Thanks.

Hi, I'm someone. :)
I've opened #3256518: Remove drupal_js_defaults() in D10 instead of D11 as the requested follow-up.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Issue isn't fixed yet, but we've more or less decided in #3246278: [policy] Define allowed 9.4.x/10.0.x changes to treat 9.4.x like a normal minor for deprecations - most of the 10.0.x blockers involve new deprecations, and we can't yet rule out having to release a 9.5.x.

Status: Fixed » Closed (fixed)

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

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented

Added #3256518: Remove drupal_js_defaults() in D10 instead of D11 to the CR and published the CR.