Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/Template/Snippet.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,58 @@ public static function end(): void
echo static::$current?->render();
}

/**
* Renders the given template.
* This method supports the use of layout snippets inside the template.
*
* @param string|null $file Path to the template file.
* @param array $data Data available inside the template.
* @param array $layoutData Data available inside any potential layout snippet.
* @return string The rendered content of the given template file.
* @throws Throwable
*
* @see Tpl::load() Base implementation without support for layout snippets.
*/
public static function load(
string|null $file = null,
array $data = [],
array $layoutData = [],
Comment on lines +151 to +152
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Where do you see the need to split this up into two parameters? Instead just having one $data parameter.

Copy link
Author

@JojOatXGME JojOatXGME Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, Template::render sets $layoutData to the same value as $data, but Snippet::render keeps the parameter empty. I only set $layoutData to the same value as $data inside Template::render for backward compatibility, but it looks to me like it actually might have been a bug. (At least the resulting deviation from normal snippets seems not to be documented.)

Within Snippet::render, I cannot set $layoutData to the same value as $data because this would lead to a runtime exception. The array for $layoutData currently must not contain 'slot' nor 'slots'. However, the $data given by Snippet::render does contain 'slot' and 'slots'.

I feel like the currently intended behavior is to always keep $layoutData empty. In other words, I think forwarding $data from the template to the snippet was actually another minor bug in the existing code, which I just preserved. If my understanding is correct, I could remove this parameter and always use an empty array instead.

However, if I look at the behavior not from the perspective of the bug fix, I would actually prefer if layout snippets would forward the data from the parent. I actually considered creating a feature request for that one or two weeks ago. This would match the behavior of “Template Inheritance” in other popular template engines like Twig, Laravel's Blade Templates, Jinja, and Django's template language. However, I would consider this a new feature and therefore did not implement this behavior. To make this feature work, I would always set $layoutData to the same value as $data, implement proper merging of slot data, and tweak the precedence of data coming from this parameter vs the data explicitly provided from within the template. (The explicit content from the template should maybe take precedence for data, but not for slots.) While I would be happy to implement this behavior, I did not consider it part of this ticket. Also note that full “Template Inheritance” usually also includes a feature to call super() or @@parent to preserve the content from the extended template. This functionality would still not be covered by the implementation I have just described.

): string {
// if the template is rendered inside a snippet,
// we need to keep the "outside" snippet object
// to compare it later
$outsideSnippet = Snippet::$current;

// load the template
$template = parent::load($file, $data);

// if last `endsnippet()` inside the current template
// has been omitted (= snippet was used as layout snippet),
// `Snippet::$current` will point to a snippet that was
// opened inside the template; if that snippet is the direct
// child of the snippet that was open before the template was
// rendered (which could be `null` if no snippet was open),
// take the buffer output from the template as default slot
// and render the snippet as final template output
if (
Snippet::$current === null ||
Snippet::$current->parent() !== $outsideSnippet
) {
return $template;
}

if (Snippet::$current->slots()->count() === 0) {
// no slots have been defined, but the template code
// should be used as default slot
return Snippet::$current->render($layoutData, [
'default' => $template
]);
}
// swallow any "unslotted" content
// between start and end
return Snippet::$current->render($layoutData);
}

/**
* Closes the last openend slot
*/
Expand Down Expand Up @@ -247,6 +299,7 @@ public function render(array $data = [], array $slots = []): string
// custom data overrides for the data that was passed to the snippet instance
$data = array_replace_recursive($this->data, $data);

// load the template representing this snippet
return static::load($this->file, static::scope($data, $this->slots()));
}

Expand Down
35 changes: 1 addition & 34 deletions src/Template/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use Exception;
use Kirby\Cms\App;
use Kirby\Filesystem\F;
use Kirby\Toolkit\Tpl;
use Stringable;

/**
Expand Down Expand Up @@ -155,39 +154,7 @@ public function name(): string
*/
public function render(array $data = []): string
{
// if the template is rendered inside a snippet,
// we need to keep the "outside" snippet object
// to compare it later
$snippet = Snippet::$current;

// load the template
$template = Tpl::load($this->file(), $data);

// if last `endsnippet()` inside the current template
// has been omitted (= snippet was used as layout snippet),
// `Snippet::$current` will point to a snippet that was
// opened inside the template; if that snippet is the direct
// child of the snippet that was open before the template was
// rendered (which could be `null` if no snippet was open),
// take the buffer output from the template as default slot
// and render the snippet as final template output
if (
Snippet::$current === null ||
Snippet::$current->parent() !== $snippet
) {
return $template;
}

// no slots have been defined, but the template code
// should be used as default slot
if (Snippet::$current->slots()->count() === 0) {
return Snippet::$current->render($data, [
'default' => $template
]);
}

// let the snippet close and render natively
return Snippet::$current->render($data);
return Snippet::load($this->file(), $data, $data);
}

/**
Expand Down
18 changes: 18 additions & 0 deletions tests/Template/SnippetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,24 @@ public function testRenderWithoutClosingAndMultipleSlots()
$this->assertSame("<h1>Layout</h1>\n<header>Header content</header>\n<main>Body content</main>\n", $snippet->render());
}

/**
* @covers ::render
*/
public function testRenderOpenLayoutSnippet()
{
// all output must be captured
$this->expectOutputString('');

new App([
'roots' => [
'snippets' => static::FIXTURES
]
]);

$snippet = new Snippet(static::FIXTURES . '/with-layout.php');
$this->assertSame("<h1>Layout</h1>\nMy content\n<footer>with other stuff</footer>\n", $snippet->render());
}

/**
* @covers ::render
*/
Expand Down