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.
Problem/Motivation
Part of #3275851: [META] Fix PHP 8.2 dynamic property deprecations
This issue intended to define all undefined properties on base classes added in #3299853: Apply #[\AllowDynamicProperties] attribute to base classes to make PHP 8.2 log size sane
Proposed resolution
Fix all child issues to safely remove the attribute
Remaining tasks
core/modules/views/src/ResultRow.php
#3275858: View's ResultRow uses deprecated dynamic propertiescore/lib/Drupal/Core/Extension/Extension.php
#2024043: Add Module, Theme, Profile, and Extension value objectscore/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
#3299857: [PP-1] Remove AllowDynamicProperties attribute from ConfigEntityBasecore/modules/views/src/Plugin/views/PluginBase.php
#3299858: Remove AllowDynamicProperties attribute from views/PluginBasecore/modules/views/src/Plugin/views/join/JoinPluginBase.php
#3299859: Remove AllowDynamicProperties attribute from JoinPluginBase
User interface changes
no
API changes
no
Data model changes
no
Release notes snippet
no
Comment | File | Size | Author |
---|---|---|---|
#17 | 3299855-17.patch | 592 bytes | andypost |
Comments
Comment #2
andypostAdded #3299857: [PP-1] Remove AllowDynamicProperties attribute from ConfigEntityBase
Comment #3
andypostAdded remaining #3299858: Remove AllowDynamicProperties attribute from views/PluginBase and #3299859: Remove AllowDynamicProperties attribute from JoinPluginBase
Comment #4
andypostAs attributes added
Comment #5
andypostRe-parented as current one suppose as fixed #3283358: [META] Make Drupal 9/10 compatible with PHP 8.2
Comment #6
longwaveMaybe even that parent can be closed now we are hopefully green on PHP 8.2? This issue technically doesn't have to be completed until PHP 9.
Comment #7
andypostGreat point! Let's introduce new tag
Comment #8
effulgentsia CreditAttribution: effulgentsia at Acquia commentedWhy are we assuming that PHP 9 will deprecate the
#[\AllowDynamicProperties]
attribute? Is there any official statement about that? I have not seen that.I think this meta issue is still worth doing, but I'm unclear as to why it's a requirement for either PHP 8.2 or PHP 9.0 compatibility.
Comment #9
Berdir> Why are we assuming that PHP 9 will deprecate the #[\AllowDynamicProperties] attribute? Is there any official statement about that? I have not seen that.
I remember seeing something about this being discussed. but that might be outdated. https://wiki.php.net/rfc/deprecate_dynamic_properties explicitly states that this will not be removed, at least that's how I interpret it.
And yes, it's definitely not a PHP 8.2 blocker.
But many of the attributes that we added also cause bad DX (e.g. the undocumented $entity->original property), so in most cases, we do want to get rid of it.
Comment #10
catchYeah so far there's no indication that they'll deprecate it, but it doesn't seem impossible that they will, so cutting down the number of places it's used like $entity->original seems good. But we might want to leave it for things like plugin annotations until we actually hear it's going to be deprecated if it's complicated to remove there.
Comment #11
longwaveI think I initially misunderstood this attribute as hiding the deprecation in PHP 8.x and that the hard error would occur in PHP 9 either way. But yes for better DX we should declare all properties where possible.
Comment #12
andypostLots of fixes could be extracted from #3295821: Ignore: patch testing issue for PHP 8.2 attributes
But things like
ResultRow
hopefully could be caught withphpstan
or other static analysis tools just not clear how to apply it for contrib and custom codebase or provide a "shim" as we have few 10.x minors aheadComment #13
andypostqueued for all 5 issues patches to get baseline for failed tests
Comment #14
andypostForst one is in #3299859: Remove AllowDynamicProperties attribute from JoinPluginBase
Comment #15
andypostone more commited #3299858: Remove AllowDynamicProperties attribute from views/PluginBase
Comment #16
andypostordered list in IS
Comment #17
andyposttesting patch for
UserSession
Comment #18
andypostFiled child issue for it #3344729: Remove AllowDynamicProperties attribute from UserSession