applyFallthroughAttributes doesn't apply when a <?php preamble is present

[iamdadmin]

Tempest version

3.3.1 dev-main

PHP version

8.5

Operating system

Linux, macOS

Description

applyFallthroughAttributes uses /^<(?<tag>[\w-]+)/ without the m (multiline) flag which anchors to the absolute start of the viewComponent.

For x-form-view.php, x-input-view.php, x-submitview.phpand any of the other built-ins or self-created components with a<?php` preamble the regex never matches, and fallthrough attributes silently don't apply.

I've extended the Fixtures and test to demonstrate this - do I have a shot at Perfect Storm?

fallthrough-preamble.view.php
x-fallthrough-preamble-dynamic-test.view.php
x-fallthrough-preamble-test.view.php

FallthroughAttributesTest.php

[iamdadmin]

I would however argue, that we actually should remove applyFallthroughAttributes and have them remain as variables which are used, or not, by the developer, in-line with closure semantics from Brent's comments in #2026 due to collision and lack of ability to turn it on or off on a case-by-case basis. In the case that developers and this applies to me too, where all my .view.php have a <?php block to at least docblock props, it would be breaking behaviour to fix it, so removing it may be less breaking, depending.

Consider if someone is using a Class Variance utility like my own tweaked https://github.com/iamdadmin/vewe-classvariance or any of the other implementations, the developer needs to be able to pass $class into that for collision treatment, whereas if class is automatically fallthrough, it will be merged inline and won't get collision treatment.

[iamdadmin]

Note: this bug still exists in 3.8 even with the recent refactor in #2076 although with the changes to code it occurs now for different reasons.

Current functionality

1. Copy, id, style, class from (effectively) ImmutableArray $attributes if they exist and merge them to the matching attributes in the first tag in the file, if this is a ViewComponentElement it obviously ends up in the child's $attributes, if it's a GenericElement it's spread into the rendered code.

The previous iteration of the bug is that this doesn't happen if the first tag in the file also isn't the first character in the file, i.e. if a PHP pre-amble block is present. However, this likely means that the bug-fix itself is a breaking change for many, as anyone with a <?php block at the top of a view file won't ever have had this working. Even if a <?php block was just added for Docblock purposes, it means that the Fallthrough wouldn't have applied. The 3.8+ iteration of the bug is in the targeting logic, which still fails if there's a PHP preamble. Tests pass, because the test files for applyFallthroughAttributes don't have PHP preambles :)

It's also hard-coded mandatory on, possibly forcing an overwrite for anything configured by the developer as it uses preg_replace to apply, and not configurable by the dev to include or exclude a given attribute, so just fixing it outright may cause more problems.

Proposed functionality in the PR I am writing

1. Copy, id, style, class from (effectively) ImmutableArray $attributes if they exist and INSERT (changed from MERGE) them to the first tag in the file, allowing for a <?php preamble
2. Except when

• id is manually configured on the first tag in the file by a developer, in which case, do not overwrite id
• style is manually configured on the first tag in the file by a developer, in which case, do not overwrite style
• class is manually configured on the first tag in the file by a developer, in which case, do not overwrite class
  OR except when
• The new ApplyAttribute I am proposing is present which implies developer control and responsibility

3. The ApplyAttribute accepts ImmutableArray $attributes which can be filtered with any of the ImmutableArray methods i.e. $attributes->diffKeys('list', 'of', 'keys', 'you', 'exclusively', 'want') or $attributes->intersectKeys('list', 'of', 'keys', 'you', 'want', 'to', 'exclude') - or a regular PHP array[] containing key => value pairs of string or boolean attributes to spread

This specifically removes the automatic merge attempt, replacing it with an "insert only if not present" approach, giving developer control over this functionality.

Benefits

• fixes the bug
• applies a back-off to the automatic fallthrough by not clashing with manually-configured instances of id, style, and class, which should avoid the fix being breaking in cases where id, style or class are already manually configured with :class="$class ?? null" or whatever other logic is in use
• adds a method of controlling which attributes are spread on the resultant GenericElement by utilising the :apply attribute