The code for locale_js_alter() contains the following comment.

  // @todo Remove this in https://www.drupal.org/node/2421323.
  $files = [];
  foreach ($javascript as $item) {
    if (isset($item['type']) && $item['type'] == 'file') {

      // Ignore the JS translation placeholder file.
      if ($item['data'] === 'core/modules/locale/locale.translation.js') {
        continue;
      }
      $files[] = $item['data'];
    }
  }

Given that #2421323: Parse js files from library definitions during rebuild to minimize variable_set() calls has been closed time ago, I guess that either:

  • The comment needs to be removed
  • The code needs to be removed

I quickly read the other issue, but I cannot say which is the correct action.

CommentFileSizeAuthor
#14 3092672-14.patch568 bytes_utsavsharma
#14 interdiff_8-14.txt568 bytes_utsavsharma
#8 drupal_remove_TODO_comment_3092672_8.patch539 bytesavpaderno

Comments

kiamlaluno created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 8.8.x-dev » 8.9.x-dev

I apologize: The other issue's status is Needs work, but it's marked for Drupal 7. I don't see any commit done for Drupal 8, although the issue was once marked for Drupal 8, so I am not really sure what this means.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

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

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

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.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Category: Bug report » Task
Issue tags: +Bug Smash Initiative

Updating category in basis of last comment

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new drupal_remove_TODO_comment_3092672_8.patch539 bytes

Since #2421323: Parse js files from library definitions during rebuild to minimize variable_set() calls has been created in 2015 and it has never had a Drupal 8 patch, I take the intention wasn't to change Drupal 8 code and that comment is a left-over from Drupal 7.

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.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.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Think I agree with #8, this comment appears to be carried from D7 for an issue that wasn't intended for D8

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove the comment in code suggesting there is code to be removed or remove the code » Remove outdated @todo from the Locale module
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll
Related issues: +#2607376: Remove on-demand JavaScript translation parsing and do everything on rebuild, +#2421323: Parse js files from library definitions during rebuild to minimize variable_set() calls, +#1677026: Less aggressive locale js file rebuilds

Thanks @smustgrave for digging this issue out. I reviewed #2421323: Parse js files from library definitions during rebuild to minimize variable_set() calls and in #6 of that issue @catch specifically descoped it from 8.x+.

For posterity, the new direction is #2607376: Remove on-demand JavaScript translation parsing and do everything on rebuild.

Unfortunately, this does not apply to 10.1.x, so we legitimately need a reroll. (#8 can still be used for the 9.5.x backport, as documentation fixes are eligible for backport.)

Saving credits. Thanks!

_utsavsharma’s picture

_utsavsharma commented
StatusFileSize
new interdiff_8-14.txt568 bytes
new 3092672-14.patch568 bytes

Rerolled patch for 10.1.x.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs work » Needs review
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -Needs reroll
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Change is simple enough.

Thanks @xjm for the additional details.

  • xjm committed 90c4f182 on 10.1.x 
    Issue #3092672 by _utsavsharma, apaderno, xjm, smustgrave: Remove...

  • xjm committed b6eb5897 on 10.0.x 
    Issue #3092672 by _utsavsharma, apaderno, xjm, smustgrave: Remove...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks @_utsavsharma and @smustgrave.

I committed #14 to 10.1.x, and #8 to 10.0.x and 9.5.x. Thanks!

  • xjm committed 030fb156 on 9.5.x 
    Issue #3092672 by _utsavsharma, apaderno, xjm, smustgrave: Remove...

Status: Fixed » Closed (fixed)

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