Problem/Motivation
#3468502: sebastianbergmann/comparator:5.0.2 Introduces (for Drupal) breaking changes was necessary because sebastianbergmann/comparator made changes.
Those changes were very superficial (only type annotations in docblocks that go beyond PHP's native yet simplistic array in actual code), but enabled sebastianbergmann/comparator to continue to work in unit tests.
However, projects that are developed with a higher PHPStan level than core — for example, Canvas is developed using level 8 — run into a remaining failure:
102 Parameter #1 ...$cache_contexts of static method Drupal\Core\Cache\Cache::mergeContexts() expects list<str
ing>, array<string> given.
🪪 argument.type
💡 array<string> might not be a list.
102 Parameter #2 ...$cache_contexts of static method Drupal\Core\Cache\Cache::mergeContexts() expects list<str
ing>, array<string> given.
🪪 argument.type
💡 array<string> might not be a list.
— https://git.drupalcode.org/project/canvas/-/jobs/7375701
because
Steps to reproduce
PHPStan level 8, and something like:
Cache::mergeContexts([], User::load(1));
(or really, anything that implements CacheableDependencyInterface)
Proposed resolution
Adjust docs for \Drupal\Core\Cache\CacheableDependencyInterface::getCacheContexts()
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3559209
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:
Comments
Comment #3
wim leersComment #4
smustgrave commentedmind updating the MR for 11.x and I can mark it for you
Comment #7
ritarshi_chakraborty commentedComment #9
smustgrave commentedThanks @ritarshi_chakraborty but it's usually good to let the previous maintainer have a little time to respond, especially with such a small change.
Comment #10
ritarshi_chakraborty commentedThanks for pointing that out — I honestly thought the previous maintainer might be inactive, so I went ahead with the change. I’ll definitely keep your advice in mind and make sure to give a bit more time for a response next time. Appreciate the guidance! 🙏
Comment #11
quietone commentedMaybe the title can be shorter
Comment #12
smustgrave commentedNo big deal at all! Just wanted to point out for next time. Thanks!
Comment #13
borisson_The title is cache tags and contexts, but the pull request only changes the docs for
getCacheContexts. This is becausegetCacheTagsis already the right type.I updated the title because of that. RTBC++
Comment #14
longwaveBackported down to 10.6.x as a docs fix.
Committed and pushed 52899c7c616 to main and e657859095a to 11.x and 53f356512ad to 11.3.x and a42213654ec to 10.6.x. Thanks!