Problem/Motivation

#3251378: Block Plugins don't store their context mappings on submit added (most likely accidentally) a return type to this method without BC. This will break any module with a block plugin that implements that method. It's likely this will affect many sites.

Steps to reproduce

Install 11.2
Install a module such as entity_hierarchy_microsite or any other that has a block plugin that implements the method
Try to load the site

Proposed resolution

Remove return type for now.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3537720-nr-bot_r1cgbunl.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3537720

Command icon 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:

About issue forks

Comments

acbramley created an issue. See original summary.

acbramley opened merge request !12824

acbramley’s picture

acbramley commented
Priority: Normal » Major
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Straight forward enough.

mstrelan’s picture

mstrelan commented

For completeness, if anyone is wondering what happens to modules that may have already updated to the new method signature. The answer is nothing, there is no harm returning void from a subclass if the parent has no return type. https://3v4l.org/B8MS6

smustgrave’s picture

smustgrave commented

Out of my own curiosity how would we go about adding this?

mstrelan’s picture

mstrelan commented

I think base classes are similar to interfaces, so see #3333824: Enable existing interfaces to add return type hints with a deprecation message for implementors

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove return type from BlockBase::submitConfigurationForm » Remove return type from BlockBase::submitConfigurationForm()
xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove return type from BlockBase::submitConfigurationForm() » [regression] Remove return type from BlockBase::submitConfigurationForm()

  • xjm committed dfd95217 on 11.x 
    Issue #3537720 by acbramley, mstrelan: Remove return type from BlockBase...
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3537720-nr-bot_r1cgbunl.txt91 bytes

The 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.

  • xjm committed 6faec7c9 on 11.2.x 
    Issue #3537720 by acbramley, mstrelan: Remove return type from BlockBase...
xjm’s picture

xjm
she/her
Primary language English
commented
Version: 11.x-dev » 11.2.x-dev
Status: Needs work » Fixed
Issue tags: +11.2.3 release notes

Good catch, thanks @acbramley and @mstrelan!

I would ask if we should have a followup to add it back, but since this is a specific implementation of a larger API, it should be handled in our meta issues to add these typehints in a major version.

Committed to both 11.x and 11.2.x (separately, since it requires an updated baseline), as a hotfix for an unintentional BC break. It's also worth mentioning in the 11.2.3 release notes that we've resolved this regression.

Crossposted with the bot. No, bot. Bad bot.

xjm’s picture

xjm
she/her
Primary language English
commented

Fixing credit for me as the committer, which the bot ate.

xjm closed merge request !12824

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.