Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.New deprecations at 9.1 due to [#2273381} committed on 16 Oct
202x: Drupal\Core\Plugin\ContextAwarePluginBase is deprecated in drupal:9.1.0 and will be removed before drupal:10.0.0. Instead, use \Drupal\Core\Plugin\ContextAwarePluginTrait. See https://www.drupal.org/node/3120980
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3178551-7.context-aware-plugin-trait.patch | 5.55 KB | jonathan1055 |
Comments
Comment #2
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedHere's a patch for .travis.yml to cater for the extra 202 deprecations and allow the tests to pass. Then any new deprecations will show up when we get new failures.
Comment #3
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedMarking RTBC so that this can be committed. Then we can work on the actual correction in due course.
Comment #5
TR CreditAttribution: TR commentedCommitted #2 to remove the deprecations for now.
I saw that CR, and it worries me because it was handled in a way typical for core issues - major potentially disruptive changes were slipped in as incidental to the real issue, and were not discussed beforehand.
The CR and the issue is about "Deprecate passing context values to plugins through configuration and remove ContextAwarePluginBase (component & core) in favor of a new trait"
This sounds like a trivial structural change that shouldn't pose a problem for anyone. Therefore, little thought has been put into approving and committing that change. If that's all there was to it, then it wouldn't need much thought or discussion. But at the end of the CR, as an afterthought, there's this statement: "passing context values to plugins via configuration has been deprecated". That little afterthought may be fatal for Rules, depending on what it means (which is not explained) and how it is implemented, because storing context values in configuration is fundamental to Rules. The CR doesn't discuss this. Now maybe this won't affect us, but it makes me nervous - Rules is the major consumer of Context, so Rules suffers all the consequences of "trivial" changes that core makes to the Context code. We saw that with EntityContextDefintion (which we still have to get fixed in core, BTW), and I'm afraid this might be another "trivial" change which costs us months or years of extra work.
Comment #6
TR CreditAttribution: TR commentedContextAwarePluginTrait is a mess - most of the methods are entirely undocumented, and it assumes that the class using it extends certain unspecified base classes and implements certain unspecified interfaces. Plus it changed the visibility of at least one interface-defined method, if you happen to be using that interface.
The change record is incomplete and leaves out some essential information.
But the biggest problem for the future is probably going to be what I mentioned above: "passing context values to plugins via configuration has been deprecated".
I will have to start a core issue for all of this - just writing it up is going to be hours of work, and who knows if it will ever be fixed.
Regardless, here's a work-around that I think will take care of the immediate deprecation problem.
@jonathan1055, can you want adjust the deprecation warnings in .travis.yml ?
Comment #7
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedYes all of the "Drupal\Core\Plugin\ContextAwarePluginBase is deprecated" messages have been removed with your patch. As requested I have added an update to .travis.yml with the new deprecation numbers. I also removed testing at Drupal 8 and added other improvements which I already had in the pipeline, but did not raise a separate issue as I know Travis is not part of your main testing infrastucture. However, if you are going to commit a change to .travis.yml then it would be nice to have these new changes in too.
You can see we have a new deprecation at 9.2
The "core/jquery.ui" asset library is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3067969. I will create a new issue for this.Comment #8
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedSet new parent for this issue #3257972: [meta] Rules deprecated code D9 -> D10
Also raised #3257978: The "core/jquery.ui" asset library is deprecated in drupal:9.2
Comment #9
TR CreditAttribution: TR commentedWhat about D10 testing, is that something you would want to add to the travis configuration now? It's currently working on DrupalCI, testing against D10 on a weekly basis in order to give us early warning.
Comment #10
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThanks but let's go with what we have at the moment, and get the changes in. It's good that you have set up a weekly D10 test on drupal.org.
I will look at adding it to travis in due course.
ps. Sorry about causing you extra work on #3257978: The "core/jquery.ui" asset library is deprecated in drupal:9.2. I just created the issue to record the fact, I did not expect someone to raise a MR and start coding changes without discussing it first on the issue.
Comment #12
TR CreditAttribution: TR commentedThanks. Committed.
No problem with #3257978: The "core/jquery.ui" asset library is deprecated in drupal:9.2 - if someone wants to help out that's good with me! Hopefully they'll take the review and make the changes, if not at least we now have more relevant information there, should someone else want to get involved.
Comment #13
TR CreditAttribution: TR commented