Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
From #3497021: [PP-1] ConfirmFormInterface::getFormName() serves no purpose in ConfirmFormBase:
I discussed with @longwave - the thing that's tricky here is that we are only removing this because it is dead code but the value is still being used by contrib and these modules will break silently because the deprecation we're adding here is only for the method. The current implementation will break modules using
$form_state->getValue('confirm')immediately. Obviously form arrays are not part of the BC promise - see https://www.drupal.org/about/core/policies/core-change-policies/frontend... - but I think we can/should do better. I think we should consider adding a#deprecatedkey to the form arrays so we can issue a deprecation when calling$form_state->getValue()for a specific key. I think doing this would allow us to remove old stuff like this and get modules to update without breaking stuff.
Steps to reproduce
Proposed resolution
One thing to consider is that $form_state->getValue() can be passed nested keys as arguments, so the implementation of #deprecated might need to account for that.
Remaining tasks
When fixed, update the "How to deprecate' policy page.
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3519300
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:
changes
Comments
Comment #2
godotislateComment #3
joachim commentedDoes something for this need to be added to the core BC policy too?
Comment #4
catchOnce it exists we should add to the 'how to deprecate' page. For me this is a nicety that doesn't change the BC policy for forms though - there are plenty of form changes that would not be able to use this and we don't want to preclude ourselves from doing them. I also don't think we'd bother using this on obscure admin forms either.
Comment #5
quietone commentedI agree with catch. Adding tag and item to the remaining tasks about updating the 'how to'.
Comment #6
godotislateAnother thing to consider whether it's worth covering for the deprecation: Some form submit methods use
$form_state->getValues(), then reference a specific key on the array, e.g.$form_state->getValues()['confirm'];or$values = $form_state->getValues(); $confirm = $values['confirm'];.Comment #8
godotislateMR is ready.
Can't trigger deprecations for usages of
getValues()after all, so focusing strictly ongetValue().CR: https://www.drupal.org/node/3521264
Comment #9
godotislateUpdated deprecation version to 11.3 and refactored the test a little.
Comment #10
larowlanThis looks great - just had one question about some extra coverage for subform state which this code will run for, but we don't have any tests for in the MR yet
Comment #11
godotislateRebased for the merge conflict.
Subform state should be accounted for now, and some logic issues fixed, with additional test coverage.
Comment #12
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #13
godotislateMerge conflict because #3525331: Reuse element plugins as object wrappers around render arrays was reverted, but maybe just gonna wait to see if it gets back in soon.
Comment #14
godotislateSince #3525331: Reuse element plugins as object wrappers around render arrays still needs work, rebased this and back to NR.
Comment #15
smustgrave commentedCan we update the deprecations, sorry didn't get to this in 11.3
Comment #16
godotislateUpdated deprecation version in CR and test code to 11.4.0.
Comment #17
joachim commentedCould the array shapes stuff that Symfony is introducing help with this?
Comment #18
godotislateRe #17: I don't see how, since render arrays are a Drupal thing.
Comment #19
joachim commentedYes, but my understanding was that Symfony's array shapes gives you a way to define the schema for an array.
Comment #20
godotislatehttps://symfony.com/blog/new-in-symfony-7-4-better-php-configuration
Drupal doesn't use Symfony config AFAIK, and render arrays aren't config anyway.
Unless you are referencing something not documented there?
Comment #21
joachim commented> Drupal doesn't use Symfony config AFAIK, and render arrays aren't config anyway.
It doesn't matter about the config part! I understood the Symfony thing to mean that Symfony now has some fancy way to validate the structure of arrays -- any arrays. Which means that maybe that can be used to say something in an array's structure is deprecated. Maybe I've misunderstood though.
Comment #22
godotislateNot according to the blog post I linked. Symfony will define array shapes for Symfony config. The validation of those array shapes can be done by a separate developer tool like "PhpStorm, PHPStan, Psalm, and others."
We've been introducing some array shapes bit by bit into Drupal core, but core's PHPStan rule level is not set high enough for those to be validated.
I'm also not sure that it's reasonably feasible to provide array shapes for render arrays, since they can have arbitrary keys and properties added at various points in processing.
Comment #23
godotislateComment #24
godotislate@smustgrave were there any other comments on the MR? The deprecation versions are actually only test code, not real deprecations, so they don't need to match anything. Regardless, they have been updated.
Comment #25
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #26
godotislateRebased.
Comment #27
godotislateBumped MR/CR version numbers to 11.5.