Problem/Motivation
While working to make SectionStorage plugins context-aware, it was pointed out that context is a confusing name for what is really an array of context definitions.
The block annotation class (\Drupal\Core\Block\Annotation\Block) does not document this parameter, but it is assumed to be named that by other code.
The condition annotation class does document this parameter.
Proposed resolution
Switch from context to context_definitions
Remaining tasks
- Add
context_definitionsto both annotation classes - Add a BC layer to support the old style, with @trigger_error
- Update all calling code
- Write a CR
User interface changes
N/A
API changes
BC support for changes
Data model changes
N/A
Release notes snippet
For context-aware plugins defined by annotation, the key for specifying context definitions has changed from context to context_definitions. Interacting with the context definitions of a plugin is unchanged, see \Drupal\Component\Plugin\ContextAwarePluginInterface.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3014949-context_definitions-13-interdiff.txt | 773 bytes | tim.plunkett |
| #13 | 3014949-context_definitions-13.patch | 29.14 KB | tim.plunkett |
| #8 | 3014949-context_definitions-8-interdiff.txt | 11.7 KB | tim.plunkett |
| #8 | 3014949-context_definitions-8.patch | 28.95 KB | tim.plunkett |
| #6 | 3014949-context_definitions-6-interdiff.txt | 8.55 KB | tim.plunkett |
Comments
Comment #2
tim.plunkettComment #5
tim.plunkettComment #6
tim.plunkettNew approach, as this fix cannot wait until plugins are instantiated.
Comment #8
tim.plunkettWith that
class_implements()check,\Drupal\Tests\Core\Plugin\DefaultPluginManagerTest::testDefaultPluginManagerWithPluginExtendingNonInstalledClass()will fatal due to\Drupal\plugin_test\Plugin\plugin_test\fruit\ExtendingNonInstalledClassI think it's safe to skip that check.
Comment #9
eclipsegc commentedhah! found a good use for private, nice.
This seems super reasonable and I support this change completely.
Eclipse
Comment #10
alexpottThis change looks sensible. Not the plugin definition key matches the method name
\Drupal\Core\Plugin\Context\ContextHandler::getContextDefinitions()Let's add an
@expectedDeprecationto make that's triggered.Comment #11
dhirendra.mishra commentedComment #12
dhirendra.mishra commentedi shud add @expectedDeprecation of which method ?
Comment #13
tim.plunkettInteresting that we use
@expectedDeprecationinstead of$this->setExpectedDeprecation(), the opposite of@expectedException/setExpectedExceptionComment #14
tim.plunkettComment #15
eclipsegc commentedLooks good to me.
Eclipse
Comment #16
alexpottdoh forgot to ask for a change record. As a deprecation and a change we need one. Also probably need a release note snippet.
@tim.plunkett we added the expectDeprecation functionality to have dynamic expectations... but we've always hoped to move this upstream.
Symfony's deprecation functionality on it's own only supports the annotation at this point.
Comment #17
phenaproximaChange record added: https://www.drupal.org/node/3016699
Comment #18
alexpottCommitted bab4f6d and pushed to 8.7.x. Thanks!
Fixed unused use on commit.
Comment #20
tim.plunkettSee #3016829: Follow-up for intersection of #3014949 and #2968139
Comment #21
tim.plunkettComment #22
tim.plunkettComment #23
upchuk commentedAdding #2809305: Block Context assignment form element shows even if no options are available as related.
Comment #24
tim.plunkettNote that the patch in that issue actually passed testing, and only failed because of the deprecations. Meaning that the BC layer worked great!
Comment #26
tim.plunkett