fix(view)!: fix inconsistent fallthrough attributes and introduce apply attribute for full control#2092
Open
iamdadmin wants to merge 12 commits intotempestphp:3.xfrom
Open
fix(view)!: fix inconsistent fallthrough attributes and introduce apply attribute for full control#2092iamdadmin wants to merge 12 commits intotempestphp:3.xfrom
iamdadmin wants to merge 12 commits intotempestphp:3.xfrom
Conversation
…2040, and add ApplyAttribute allowing developer control
Benchmark ResultsComparison of Open to see the benchmark results
Generated by phpbench against commit 5b85298 |
Contributor
Author
|
Weird, I can't replicate the run style check's unused view import. Locally mago on the same version doesn't find any unused view imports, and the logs don't show where. I'll do some digging. |
brendt
requested changes
Mar 31, 2026
Contributor
Author
|
Blocked by #2094 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2040.
This is likely to be a breaking change, I've taken the liberty of writing a blog post in Markdown explaining it, which you're welcome to post, adapt, or not use as suits your needs.
If changes are needed, I can adjust as well. Here's it pasted in for all to read, I have attached it as a downloadable .md file as well for easy import to the website if you want it.
Updated-approach-for-fallthrough-attributes-in Tempest-View.md
Updated approach for fallthrough attributes in Tempest View
Tempest's view components provide a convenient automatic fallthrough of
class,style, andidfrom the call site onto the root element of a component template. Passclass="mt-4"to<x-button />and it appears on the<button>inside — no boilerplate needed.However, a simple bug in the code meant that this didn't happen consistently, and even then there was no method to control the behaviour if it was not required. The inconsistent nature of the bug - anchored on whether or not there was a PHP preamble or other content before the 'first' HTML token in the component - means that a direct fix would be a breaking change for many applications.
The Bug
The original implementation used a single regular expression anchored to the start of the compiled template string:
The
^anchor means: match only if the very first character of the file is<. For a component like this, it worked fine:But the moment a PHP preamble appeared before the first tag — even a
declarestatement or a comment — the regex found nothing, silently produced no output, and the fallthrough was lost entirely:No error. No warning. The
classyou passed from the parent simply never arrived.Tempest version 3.8.0 brought changes to the way a view component was compiled, but the bug still asserted itself. The regex was removed and replaced with a proper AST walk using
TempestViewParser::ast()- a big step forward - but the fallthrough check was anchored to the token index instead of the token type:$i === 0means: only attempt fallthrough if the first token in the parsed AST is the opening tag. However, a PHP preamble is its own token. So when one was present, the HTML element appeared at index1or later,$i === 0was false,$shouldApplyFallthroughwas false, and the fallthrough was silently dropped — exactly as before, just through different code. The same tests that failed before continued to fail locally.Why wasn't it spotted before?
There are tests for this - several in fact. However, they all used minimal fixtures, and none of which had a PHP preamble.
This update also brings updated tests, all of which include a PHP preamble, with simply a comment to hold it in the file, in order to ensure that the issue is tested for going forwards.
No developer control over fallthrough attributes
Beyond the bug, the original implementation was all-or-nothing. It always attempted to merge
class,style, andidonto the first element, with no way to opt out, no way to target a specific element, and no way to know it had run. If your component defined its ownclasson the root element, the incoming value was appended on top of it regardless.How it was resolved
Finding the first tag properly
The regex was replaced with a proper AST walk.
TempestViewParser::ast()already tokenises the template, so instead of guessing with a pattern, we iterate tokens and find the first realOPEN_TAG_STARTorSELF_CLOSING_TAG— skipping past PHP blocks, whitespace, comments, and doctypes entirely:The first valid HTML token is found regardless of what precedes it in the file. PHP preambles,
declare(strict_types=1), comment blocks — none of them interfere.Respecting what the component already declares
The previous behaviour unconditionally merged incoming values onto whatever the root element had. The new behaviour checks first: if the root element already declares
class,:class,style,:style,id, or:id, the fallthrough for that attribute is now skipped entirely.For applications previously relying on the mandatory merge of these attributes, this is a breaking change, meaning you'll need to adjust your views to manually implement the merge. This was a necessary change however, in order to add control.
This now means a component can take ownership of any of these attributes simply by declaring them. Pass whatever you like from the call site — if the component has already configured it, the component wins:
With the above,
classis owned by the component. Passclass="something-else"from the call site and the component's expression takes precedence.idandstyle, having no declaration on the root, still fall through as before.The new
:applyattributeFor cases where you want full manual control — spreading additional attributes, filtering specific ones, or building the attribute set dynamically — the new
:applyattribute hands you that control explicitly.Placing
:applyanywhere in a component template disables automatic fallthrough for that component entirely. The$attributesvariable, anImmutableArrayof everything passed at the call site, is always available:Because
$attributesis anImmutableArray, you can filter it before spreading. To exclude specific attributes:To include only specific attributes:
You can also build the array entirely yourself in a PHP preamble and pass it in, which pairs naturally with the
:asattribute for components that conditionally render as different elements::applystringifies attribute values according to the following rules:trueemits a bare attribute name;false,null, and empty string are omitted; everything else is rendered asname="value".