[6.x] Include Tag - #15182
Conversation
|
Looking forward to using this! After a review and playing around, got a couple things I want to make sure are intentional: Blade → Antlers cascade — If I do scope punching out — Using
|
Refactored a few things so the
Added a test to codify it. It's a neat use of existing behaviors and I don't see the harm in it; it'd be a lot more work to shut this down and its kind of cool!
The existing behavior is correct IMO. The @if (Statamic::tag('include:exists')->src('cards/author')->fetch())
@endif
@if (Statamic::tag('partial:exists')->src('cards/author')->fetch())
@endif{{ if {include:exists src="cards/author"} }} ... {{ /if }}
{{ if {partial:exists src="cards/author"} }} ... {{ /if }}
|
|
Awesome, thanks man! |
jasonvarga
left a comment
There was a problem hiding this comment.
Really nice piece of work — the scoping model is the right call, the try/finally discipline around cascade and prefix state is careful, and the test suite (9 files, ~1,680 lines, one test per closed issue plus a dedicated SandboxTest for scope-escape attempts) is unusually thorough. The randomized nowdoc terminator is a good catch on its own: slot content containing a literal COMPILED; line would previously have broken out of the hoisted heredoc.
A couple of things need fixing before this ships, since once the behaviour is out it becomes BC. Details are on the relevant lines; the rest are notes below.
Notes (not line-specific)
Missing Antlers coverage for include:exists / include:if_exists. The PR description documents <s-include:if_exists src="cards/{type}" />, and RendersViews::exists() / ifExists() are new code paths for IncludeTag. Blade covers them (IncludeCompilerTest::it_forwards_exists_method_calls and it_forwards_if_exists_method_calls), but there's nothing equivalent under tests/Antlers/Runtime/Includes/. Same gap in the other direction for the src form — Antlers has test_it_renders_a_view_using_the_src_form, Blade doesn't test <s:include src="..." />.
New trait members are private in an otherwise protected trait. CompilesPartials was entirely protected; the six new members (compileSlotOutput, compileIncludeSlot, rawSlotName, isValidSlotName, compileInclude, compileViewTag) are private. compilePartial() stays protected and delegates, so the existing extension point survives — consistency nit only.
GlobalRuntimeState::captureRuntimeState() going from 3 to 4 elements is handled safely. restoreState() uses $capturedState[3] ?? true, and list-destructuring in third-party code ignores the extra element. Just flagging that I checked.
Review feedback pointed out that spread() silently dropped any :params key named after a tag option (src, when, unless, cascade, params, handle_prefix), so spreading an entry with a src field lost it with no warning. None of the control params are ever read from the spread, so the except() was not protecting anything, and it was inconsistent with prefix aliasing, which happily produced those same names. Spread keys now always become view data, and tag options only ever come from parameters set on the tag itself.
Review feedback caught that piping a slot through modifiers skipped the modifier chain entirely, while the parameters were absorbed into the slot's render data. Filtering parameters through the modifier registry was considered and rejected: prop names would become hostage to which modifiers happen to be registered (a :title prop collides with the title modifier, and any addon registering a modifier would silently change which props reach slots). The rule is now explicit and the same for default and named slots: every parameter on a slot output is a prop, and modifiers are applied by piping. The runtime coerces a Slot to its rendered content at the start of a modifier chain, so chains behave exactly as they would on any string variable, and slots are terminal values during reduction so props are never misread as modifier arguments.
Review feedback pointed out that the compiled slot output discarded a pair's inner content, so a paired slot tag in an included Blade view rendered nothing when the slot was not supplied, and there was no supported way to provide a default when the slot alias collides with a data key. The question was whether pair bodies are fallbacks or are intentionally ignored. They are fallbacks. The compiled output now renders the provided slot when there is one and the pair body when there is not, matching what Blade users would expect from components. The Antlers side keeps its existing semantics (a missing named slot pair renders nothing, and conditions provide defaults), which is now pinned as well. Compiler methods are widened to protected alongside this for consistency with the rest of the trait.
Review feedback flagged that serializing a slot baked in an eager, props-less render: a scoped slot inside a nocache region served stale content on cached requests and degraded with no signal. Slots now serialize as their original source instead of freezing. An Antlers slot stores its raw template text (already available on paired nodes as runtimeContent) along with the caller state it needs, a Blade slot stores its hoisted template, and both carry their captured scope, filtered through the same rules nocache regions already use. On the way back out the slot re-parses through the bound parser, which also restores cascade access, runtime configuration, and variable guards on replay. Storing source rather than the node graph keeps payloads to a couple hundred bytes; serialized nodes dragged the entire document in through parser back-references. Supporting changes, each pinned by the new nocache and slot tests: - Region grows a preserved-context-keys mechanism (registered by the view provider, no include knowledge in Region itself) so the include carrier keys survive region filtering; without them, compiled Blade slot output loses its context on replay and falls through to a TagNotFoundException. The same filtering is reused for slot scopes, which also keeps slot forwarding working across replays. - Named slot references resolve by instance as well as by the per-request indicator, since the indicator can never match in a replayed region. - A scope that genuinely cannot be serialized throws at cache time, naming the slot and the offending keys, rather than quietly caching wrong output. Deferred Blade component variables are resolved the same way regions already resolve them. A template matching a view name is prefixed so Blade::render treats it as content. - Closure-built slots keep the eager render, since there is no other representation, and reject late props loudly.
Naming a slot and attaching the view's params mutated the Slot instance in place. Forwarding the same instance into a nested include then clobbered the outer view's slot: the inner include's params replaced the outer ones, so rendering the slot again after the nested include showed the wrong values. Each include now works with its own clone, so every view names and parameterizes its slots without affecting anyone else holding the same slot. This came out of reviewing the slot lifecycle for the serialization feedback.
Adds coverage that came out of review discussion rather than code changes: the exists and if_exists forms work through the include tag, the src parameter form compiles on the Blade side, and a view literally named index stays reachable from both engines. The bare tag resolving to the index view intentionally matches what partial does, since a site can genuinely have an index view.
Appeases PHPStan's new static() rule; Slot isn't an extension point.
The parseView() data restore previously applied to every view render. Restoring unconditionally is arguably correct, since a view leaving data behind on the processor is the same mechanism behind the statamic#8175 leaks, but it also touches long-shipped partial behavior in edge cases. The restore is now opt-in per render and only the include tag requests it, so everything outside include keeps its existing behavior. A note on the flag covers removing it once the underlying bug is fixed for everyone. Deferred slot renders leaned on the unconditional restore: a partial rendered inside deferred slot content could clobber the enclosing view's scope once the restore became scoped. Slot output now locks processor data around the render, the same idiom the tag invocation path already uses.
Fixes #8175
Fixes #10703
Fixes #11486
Fixes #12709
Overview
This PR adds a new
includetag: a strictly-scoped alternative topartialfor rendering another view, available in both Antlers and Blade.The
partialtag automatically shares every variable from the template using it with the partial being rendered. That convenience is the root cause of a long line of historical scoping issues and little paper-cuts. Variables set inside a partial leaking back out (but only sometimes), parameters and front matter showing up in other partials rendered later on the page, and behavior changing depending on which syntax was used.Fixing these issues with the existing
partialtag would absolutely break a ton of sites, soincludeis here!How it differs from
partialpartialincludecascade="true"Passing data
You must pass data to the include tag explicitly. Parameters become variables inside the view, and you can spread an entire array using
:params. Inside the view, useparamsto check what was passed in:Use
handle_prefixto make prefixed keys likehero_titleavailable as bothhero_titleandtitle:Slots
Content between the tag pair becomes the default slot, and you can define named slots with
slot:namepairs.Slots only render when the view actually uses them, and the view can pass data back to your slot content. A view can render a slot once per item in a loop, for example:
You can also forward a slot you received on to another include:
The Cascade
Included views don't see the Cascade by default. Pass
cascade="true"when you want it:Conditionals and existence
Use
whenandunlessto control whether anything renders.existsandif_existswork the same way they do onpartial:The issues
includethey never do.view:data of the next one rendered on the page. Each include only sees its own.{{ if }}and using the?=shorthand isolate variables differently. Both behave the same withinclude.Notes for reviewers
handle_prefix, the prefix rewrites variable lookups in everything rendered inside it. The include tag suspends this while it renders, so an enclosing partial's prefix never reaches the included view. Components rendered inside such a partial still inherit the prefix. That is probably unintentional, but changing it could break existing sites, so it's left alone for now.