[4.x] Fix runtime behavior regression

[JohnathonKoster]

This PR fixes #8829.

An earlier PR #8818 introduced a behavior regression. While the behavior is technically correct, it doesn't play well with sites that were accidentally relying on the old behavior.

An example of where the old behavior before #8818 would apply incorrectly is when using the asset tag with the url parameter:

{{ asset :url="image" }}
   {{# Do your thing. #}}
{{ /asset }}

Because url also matches a modifier name, the previous over-aggressive check would catch this tag and isolate its parsing/rendering. This PR re-introduces this old behavior when a tag returns an associative array, while preserving the corrected behavior #8818 was trying to achieve.

This behavior regression is incredibly difficult to track down and correct within sites using a page builder-type setup, where its not obvious what's being included when/by-what. I've verified it on a site (related to the attached issue), but it is highly likely to impact more.

New tests have also been added to catch this behavior, but there is the possibility that it doesn't account for all of the weird previous behavior of having the over-aggressive check apply to everything.

Note: Regardless of which action is taken (reverting #8818) or accepting this PR, there will be strange behavior in some scenarios until it can be resolved in a breaking-change version. The easier scenario/workaround to document is the workaround that would be required by reverting #8818:

{{ _the_count = 1; }}

{{ collection:articles limit="3" as="oo_shiny" }}
    {{#
        This works because the runtime *can* trace the assignments across scope barriers.
        Even though the as will force a new renderer to be created, the runtime is still
        handling the iteration/looping itself, instead of the tag's parseLoop method.
    #}}
    {{ oo_shiny }}
        {{ _the_count += 1; }}
        Entry: {{ _the_count }}
    {{ /oo_shiny }}
{{ /collection:articles }}

That workaround is required when the tag does not return an associative array, but does have parameters that conflict with modifier names. This is probably the better course of action (instead of accidentally breaking behavior), but I did make an attempt to keep both things working 🙂

Note 2: Adding the ability to force isolation and opt-into the previous behavior would also be an option, but would also be a breaking change and require changes to a site's template (and would be confusing to explain when that should be done).

[jasonvarga]

Regardless of which action is taken reverting 8818 or accepting this PR, there will be strange behavior in some scenarios until it can be resolved in a breaking-change version.

What would a breaking-change solution look like? Since we're working on v5, let's just do whatever that is.

[JohnathonKoster]

Regardless of which action is taken reverting 8818 or accepting this PR, there will be strange behavior in some scenarios until it can be resolved in a breaking-change version.

What would a breaking-change solution look like? Since we're working on v5, let's just do whatever that is.

The changes in #8818 were accidentally the breaking changes (even if though they fixed an inconsistency/bug). Tag pairs that happened to have the same names as modifiers would subtly change the behavior of things at the time. The breaking change would have been to keep 8818, which is the current state of 4.x without this PR

[jasonvarga]

What's "more correct" though? The way it is now?

If we introduced a breaking change in a non-major release, that's obviously a no-no for semver. But maybe we just call a whoopsie, ask the semver gods for forgiveness, and leave it alone. It's been this way for almost 5 months. People running into it have probably adapted to it. If we were to revert it now it could probably be considered another breaking change.

[JohnathonKoster]

What's "more correct" though? The way it is now?

In my opinion, the way it is now is the correct behavior (without this revert/patch PR. Time moves fast 😅) 👍

[jasonvarga]

Okay, let's just leave it then.