Refactor trigger_error in TypedConfigManager

Left a comment but appears lost test coverage right? Let me know if I'm wrong.

I found the issue I mentioned inline, it's ✨ Allow kernel tests to fail or expect logged errors Needs work .

Maybe we can do a test, using a prophecy, somehow. Will try.

Nice update with the prophesize logger

@alexpott asked me about bc concerns given constructor property promotion here means we now have two properties that were previously untyped getting a type. The constructor type doesn't actually change in this issue because that was already type hinted, it's only the property itself.

Properties are considered internal unless they're on a base class per https://www.drupal.org/about/core/policies/core-change-policies/bc-polic... →

We sometimes/often deprecate properties before we remove them, but we don't have that many cases of adding types to existing properties. However, I think this is fine - someone would have to be setting the property to a value with the wrong type, or overriding the property with the wrong type. So this is both covered by the bc/internal policy and also seems very unlikely to actually break anything - even if someone is subclassing this class it would be unlikely that they'd be affected by the change.

I removed the constructor docs on commit.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

catch → credited alexpott → .

mondrake → closed merge request !7283

// For a typed config the data MUST exist.

If it MUST exist should this be an exception? or at least an assert()?

@longwave pointed out that we should thrown an \ InvalidArgumentException here. We replaced a non-recoverable error with a log. That's not right...

mondrake → reopened merge request !7283

Now throwing an exception.

Exception appears to be added correctly after the log.

Needs a new title and IS update given the new approach.

Updated IS.

I think we can remove the logger again - the exception is enough.

Feedback appears to be addressed.

mondrake → closed merge request !7283

Committed and pushed ea5932e490 to 11.x and 29964e3a3a to 10.3.x. Thanks!

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