Problem/Motivation

Similar to #3232018: Trigger a deprecation when using \Drupal\Core\Cache\RefinableCacheableDependencyTrait::addCacheableDependency with a non CacheableDependencyInterface object but for the renderer service.

If you call \Drupal\Core\Render\Renderer::addCacheableDependency with something that doesn't implement CacheableDependencyInterface, you end up with uncacheable pages.

Examples of bugs in core this has lead to include: #3227637: NodeController::revisionOverview is uncacheable and probably more.

Steps to reproduce

Call \Drupal\Core\Render\Renderer::addCacheableDependency with anything that doesn't implement CacheableDependencyInterface

Proposed resolution

Trigger a deprecation error if called with an object that doesn't implement CacheableDependencyInterface
In the next major version remove that deprecation error and add a type-hint (hard break).

Remaining tasks

All of the above

Issue fork drupal-3525388

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:

About issue forks

Comments

acbramley created an issue. See original summary.

acbramley opened merge request !12175

acbramley’s picture

acbramley commented
Related issues: +#3227637: NodeController::revisionOverview is uncacheable
acbramley’s picture

acbramley commented
Status: Active » Needs review
smustgrave’s picture

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

Thanks for already deprecating 11.3.

Deprecation seems correct and straight forward

LGTM

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

One comment on the MR.

acbramley’s picture

acbramley commented
Status: Needs work » Reviewed & tested by the community

Fixed, since this is a trivial change I'm going to self RTBC.

  • catch committed a2db325e on 11.x 
    Issue #3525388 by acbramley: Trigger a deprecation when using \Drupal\...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

catch closed merge request !12175

Status: Fixed » Closed (fixed)

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