ConfigFormBase::typedConfigManager() should have been removed when deprecations were removed in 11.0

Deprecation code was added in https://www.drupal.org/project/drupal/issues/3394197 🐛 [regression] The new property \Drupal\Core\Form\ConfigFormBase::$typedConfigManager conflicts with some contrib modules Fixed and later it was removed in https://www.drupal.org/project/drupal/issues/3443488 📌 Remove deprecated code from lib/Flood and lib/Form Fixed

Was just checking the code in the ConfigFormBase class and was wondering if it's safe to fully remove the method?

/**
   * Returns the typed config manager service.
   *
   * @return \Drupal\Core\Config\TypedConfigManagerInterface
   *   The typed config manager service.
   */
  protected function typedConfigManager(): TypedConfigManagerInterface {
    if ($this->typedConfigManager instanceof TypedConfigManagerInterface) {
      return $this->typedConfigManager;
    }
    return \Drupal::service('config.typed');
  }

Would it be safer to keep to just remove the condition and turn it into a getter to avoid breaking contrib or custom modules that could be using it? Like so:

/**
   * Returns the typed config manager service.
   *
   * @return \Drupal\Core\Config\TypedConfigManagerInterface
   *   The typed config manager service.
   */
  protected function typedConfigManager(): TypedConfigManagerInterface {
    return $this->typedConfigManager;
  }

Just want to make sure!

It would be safer to keep the method and just remove the condition, turning it into a getter. This way, you avoid the risk of breaking contrib or custom modules that might rely on it. By keeping it as a getter, you ensure backward compatibility.

/**
 * Returns the typed config manager service.
 *
 * @return \Drupal\Core\Config\TypedConfigManagerInterface
 *   The typed config manager service.
 */
protected function typedConfigManager(): TypedConfigManagerInterface {
  return $this->typedConfigManager;
}

This will make sure your modules continue to work seamlessly without any unexpected breaks. Does this give you a bit more confidence in your change?

The typedConfigManager() was only added for deprecation handling, when the TypedConfigManager was added to the constructor.

There's no need for it to exist in the long term.

Fix looks good, pipeline needs to re-queue failed job

I thought faux commit would do the magic after learning about composer 2.8.1 issues failing tests.
Tests are still failing which is quite unfamiliar to me

Time: 00:08.303, Memory: 8.00 MB
    
    There was 1 error:
    
    1)
    Drupal\Tests\Composer\Plugin\Scaffold\Functional\ComposerHookTest::testComposerHooks
    RuntimeException: Exit code: 1
    
    Creating a "fixtures/drupal-drupal" project at "./create-project-test"
    Installing fixtures/drupal-drupal (1.0.0)

Any idea? @andypost

Now pipelines are passing ~~ seems some issue with gitlab CI or seems failing due to composer 2.8.1(Not sure)
Moving into review

If it was missed in D11 that was definitely a mistake but think it should be deprecated properly since D11 is already out.

Leaving novice tag.

Patch to be updated for core policy change to meet the deprecated code removal → .

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

I’ve fixed the code standards issues.

The patch successfully deprecates typedConfigManager() in ConfigFormBase, ensuring that it is no longer used in Drupal core while maintaining backward compatibility until its removal in Drupal 12. After applying the patch, analysis tools like Intelephense correctly recognize the deprecation, highlighting it in the codebase. However, no warnings appear in the Drupal logs (dblog), suggesting that core no longer calls this method at runtime. To further verify this, I also created a custom module with a configuration form that explicitly called typedConfigManager() inside validateForm(). While the function was executed, no deprecation warnings were logged, reinforcing the conclusion that this method is no longer triggered within Drupal core itself but depreciation message was triggered in codebase. Overall, the patch achieves its goal without introducing regressions, and developers should ensure they update their custom code accordingly before Drupal 12.
Hence Moving the issue to RTBC.

One comment on the MR.

Hi @catch

I'd like to clarify a few doubts before making the changes.

If I remove only the redundant part, won't the comment look strange with just "There is no replacement."?

Also, I didn't understand this other part.

Also I think the replacement is to use the typeConfigManager property as is done in ::validateForm()

Now that 'typedConfigManager' is injected as a dependency, there will always be a valid object.

In that case, wouldn't the return already be correct? Am I mistaken?

Could you explain these points to me before I proceed with the changes?

@charlliequadros the change should be fine and actually more inline with other deprecations in core. While you're at it can you rebase the MR as it's 700+ commits back

If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

Hey @smustgrave,
I’ve done the rebase as you requested. Do I need to do anything else regarding the questions I asked here 📌 ConfigFormBase::typedConfigManager() should have been removed when deprecations were removed in 11.0 Active

The comment on the MR is valid

Hello Smustgrave,

Can you explain a bit what other changes are required over the Mr because as per comment the changes are not very clear to me

Do we need to remove the typedConfigManager completely? Or anything else

The 'This method is no longer used by Drupal core' is redundant here and can be removed.
Also I think the replacement is to use the typeConfigManager property as is done in ::validateForm()

Please allow the original user least 48 hours to finish it

Hi @smustgrave,

The comment on the MR didn't make much sense to me either. Could you explain that part better?

Also I think the replacement is to use the typeConfigManager property as is done in ::validateForm()

This method is no longer used by Drupal core is incorrect. If you check other deprecation messages should help

Hi @smustgrave

Sorry, but I still don’t fully understand what I need to do. Could you clarify the next step?

Either way, if it’s not possible to explain right now, I’ll keep an eye on this ticket to see how the issue gets resolved.

I think the feedback is to change as follow,

- @trigger_error('typedConfigManager() is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. This method is no longer in use instead $this->typedConfigManager to be used. See https://www.drupal.org/project/drupal/issues/3477616', E_USER_DEPRECATED);
+ @trigger_error('typedConfigManager() is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. The replacement is to use the $this->typedConfigManager as is done in ::validateForm(). See https://www.drupal.org/project/drupal/issues/3477616', E_USER_DEPRECATED);

Fixed, now I can't review.

@smustgrave Thanks!!
Please do consider to fix the pipeline also: https://git.drupalcode.org/issue/drupal-3477616/-/jobs/4763371#L394