Illegal offset type in MailchimpCampaignForm->buildMergeVarsHtml()

+++ b/modules/mailchimp_campaign/src/Form/MailchimpCampaignForm.php
@@ -735,8 +735,9 @@ class MailchimpCampaignForm extends ContentEntityForm {
+        $name = is_object($var->name) ? $var->name->render() : $var->name;

This doesn't quite make sense to me - not all PHP objects have a render() method, so is there an interface we should be checking here instead?

I ran into this with the email variable which is added in mailchimp_get_mergevars() as 'name' => t('Email Address'), So then ->name contains a TranslatableMarkup. In this case 'render'ing it makes sense?

Also this is on the only time an object *should* be returned as the ->name. The rest of the merge_vars come from the Mailchimp API using the function getMergeFields. This function returns only string for the 'name' parameter as per https://mailchimp.com/developer/reference/lists/list-merges/#read-get_li...

Patch in #1 is a much more constructive solution than I would have made. Which would be just adding an is_scalar() in buildMergeVarsHtml() to check $var->name before trying to use it as an array key. And hence not being able to use the email at all.

Unfortunately the path did not apply on 8.x-DEV, so made a re-roll (also updated the array() notation, that was why patch from #2 failed)

The patch

Works on the latest dev branch.
Please commit.

Long comment here, but I'm rubber-ducking this patch to myself to make sure it makes sense. I think Sam's initial hesitation is a good one--determining that we have ended up with an object in a mergevar is not a future-proof guarantee that the object will be renderable. The original cause of the error is that we're trying to index an array with an object, when we actually need a type that is a valid array key. But it's still fine to pass a TranslatableMarkup object in as the value in a markup item.

For these reasons, I added a check for the more general __toString magic method, which TranslatableMarkup implements through ToStringTrait. My understanding is that casting strval() on a TranslatableArray should get us the rendered value anyway, but with a check that's less dependent on the render method specifically.

If there is a change to the API/library such that we end up with a non-renderable object in the merge, we have three choices: ignore it and silently continue the loop; run a try/catch and surface the error in a nonfatal way; or throw an uncaught warning, so basically what we had before. Try/catch seems like expensive overkill and I don't want to bury a breaking change like that, so I think the third option is probably best for this case, and would ideally lead back to this issue for context on a search.

firfin, if you can try this patch with the case you described in #4 I'd really appreciate it. VladimirAus, if you have a similar test case your review would be helpful as well.