Passing CustomFields as array to ViewableData::renderWith doesn't pass data in included templates

[wernerkrauss]

Affected Version: 4.2

Steps to reproduce:

E.g. with SilverShop, Emails are created with custom data like:

        $email->setData(
            [
                'PurchaseCompleteMessage' => $completemessage,
                'Order' => $this->order,
                'BaseURL' => Director::absoluteBaseURL(),
            ]
        );

I can call $Order in my main email template, but for some reason when I have a template with an include I cannot get the order object in my included template unless I pass it manually:

<% include OrderEmail Order=$Order %>

This seems to be related to this

silverstripe-framework/src/View/ViewableData.php

Lines 397 to 402 in 4d14e9b

	if ($customFields instanceof ViewableData) {
	$data = $data->customise($customFields);
	}
	if ($template instanceof SSViewer) {
	return $template->process($data, is_array($customFields) ? $customFields : null);
	}

If I pass the custom data wrapped in ArrayData it works as expected. Is this a bug or a feature?

[kinglozzer]

Yep, that is expected behaviour (currently), see Hamish’s explanation for why here: #2797 (comment)

[wernerkrauss]

@kinglozzer but why is $customFields working different if it's an array or an ArrayData / ViewableData? It's not just "scope" like $Top, it's custom data I pass to the template.

passing $customFields as ArrayData: can access it in an included template
passing $customFields as php array: cannot access it... but why???

Where should we fix this? In ViewableData? In Email::setData()? Convert to ArrayData if we pass an array to it?

Or just in our custom code, e.g. silvershop's OrderEmailNotifier?

[kinglozzer]

If I pass the custom data wrapped in ArrayData it works as expected.

Sorry @wernerkrauss, I missed that line!

So it looks like ViewableData instances get folded into the main $data variable, but arrays are just passed as “overrides” which aren’t preserved when jumping down into an include. I think we should make them behave the same when passed to ViewableData::renderWith(), could you make a PR against 4 to see if it breaks any unit tests? We’ll probably need to carefully review it anyway but that would be a good starting point 😄