The module fails in Drupal Core 11.x dev branch due to deprecated LibraryDiscovery Class

Created on 2 September 2024, 4 months ago
Updated 3 September 2024, 4 months ago

Problem/Motivation

The Blazy module is failing in the Drupal Core 11.x dev branch when used with the contributed modules that have added Blazy as a production or development dependency. The issue arises because the LibraryDiscovery class, which the module relies on, has been deprecated in Drupal Core 11.x and is scheduled for removal in Drupal Core 11.1.x. This deprecation is causing the our CI pipeline to fail when running against the 11.x dev branch, specifically the NEXT_MAJOR branch.

Following is the change record:
https://www.drupal.org/node/3462970

Proposed resolution

Replace the usage of the LibraryDiscovery class with LibraryDiscoveryCollector in the src/Asset/Libraries.php file.
The LibraryDiscoveryCollector class should work with both current and previous Drupal Core versions, ensuring backward compatibility.

📌 Task
Status

Closed: won't fix

Version

3.0

Component

Code

Created by

🇮🇳India vishalkhode

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @vishalkhode
  • 🇮🇳India vishalkhode

    vishalkhode changed the visibility of the branch 3471672-the-module-fails to hidden.

  • 🇮🇳India vishalkhode

    vishalkhode changed the visibility of the branch 3471672-the-module-fails to active.

  • Merge request !28Fixed the library.discovery deprecation. → (Open) created by vishalkhode
  • Pipeline finished with Failed
    4 months ago
    Total: 194s
    #271923
  • Pipeline finished with Success
    4 months ago
    Total: 211s
    #271930
  • Status changed to Needs review 4 months ago
  • 🇮🇳India vishalkhode

    There are couple of failures in Next Major CI, which are un-related to this issue, hence, I've not fixed this as part of this ticket. But If needed/requested, I'll fix those here. Hence, moving it to Needs Review.

  • Issue was unassigned.
  • Status changed to Closed: won't fix 4 months ago
  • 🇮🇩Indonesia gausarts

    Thank you.

    Deprecation jobs are normally taken care of by Drupal Rector.

    Manual works had been proven to fail and cause fatal errors somewhere as seen at Slick project, etc.. Not always, but a few times. Even when they were harmless at first glance. We should avoid wasting energy for manual works whenever bots can do better and safer jobs, specific for depreciation issues.

    With due respects, I didn't want to repeat terrible experiences. Worst when the patchers didn't care enough to reply to my questions on the affected subject matters. I am a very patient man, I asked them six times very patiently and respectfully, and each question had never been replied properly, and when later I applied their patches, boom, I got errors somewhere they didn't even bother to answer in the first place. He repeated the same arrogant patches without explanation at this module, and I could no longer be patient. People who never read the entire story might think I was impatient, but rest assured I am a very patient man until the seventh annoying repetition. Be warned, I might reduce it to the third like other normal people once in a while, though.

    You are good at communicating the problems and potential regressions, thank you! The previous patchers are horrible at communications. They should learn from you how to communicate an issue.

    Please focus your efforts on helping other areas instead. Yours is useful for those in urgent need whenever bots come too late to party.

    Appreciated, nevertheless.

  • 🇮🇩Indonesia gausarts

    I just have time to look back, so for documentation purposes:

    https://git.drupalcode.org/project/drupal/-/blob/11.x/core/core.services.yml?ref_type=heads#L1691
    library.discovery.collector:
        arguments: ['@cache.discovery', '@lock', '@library.discovery.parser', '@theme.manager']
        class: Drupal\Core\Asset\LibraryDiscoveryCollector
        deprecated: The "%service_id%" service is deprecated in drupal:11.1.0 and is removed from drupal:12.0.0. Use LibraryDiscovery instead. See https://www.drupal.org/node/3462970

    Meaning library.discovery.collector is deprecated for library.discovery. Not the opposite.

    You appeared to misunderstand the CR, and your patch did the opposite, and so it will break D12.

    As I said manual deprecation works are prone to fatal errors, some due to misunderstanding, the rest are carelessness. Hopefully you don't post this type of issues anywhere else, or you'll break them. If you did, be sure to close them out.

    But don't take it so hard, just so we are looking at things right. We all made mistakes. What matters, we correct them whenever we can.

  • 🇮🇳India vishalkhode

    Hi @gausarts
    Thanks for your suggestions, duly noted. I realize that I mistakenly updated the service from library.discovery to library.discovery.collector, which is now deprecated. I believe this would only break in Drupal 12 (which hasn’t been released yet), correct? That’s why we have the review system on drupal.org—to ensure that reviewers can catch these mistakes, suggest changes, and help get them fixed before merging. I agree that mistakes happen, but that’s where the review process comes in to catch and correct them.

    The bigger issue here, in my opinion, is not that developers make incorrect changes in their MRs—nobody sets out to do that. The main problem is the absence of CI for this module. If we had CI, it would automatically test the module across all supported Drupal Core versions, flagging any test failures, deprecations, or code standards issues. Without this in place, it’s easy to merge changes that haven’t been fully tested across the different core versions.
    Thanks.

  • 🇮🇩Indonesia gausarts

    You are correct. Do not hesitate to post any more improvements you have in mind in another thread.

    That would be very much appreciated!

Production build 0.71.5 2024