PR 10226 introduced bug with nav tag when using select

[martyf]

Bug description

When using the select param on the nav tag in 5.19, it appears keys get cached internally across multiple calls especially when used within a loop.

How to reproduce

Reproducible with this repo: https://github.com/martyf/statamic-issue-bulk-augmentor

Within a nav tag, the code is calling a Tag that returns the context's entry_id.

On 5.19, you'll see the "Expected" results are not returned - the entry_id is the same for every loop.

Change the composer.json statamic/cms version to 5.18 and you'll see the entry_id be correct for every loop.

On 5.19, if the changes to src/Data/BulkAugmentor.php from #10226 are reverted, the issue disappears (however I'm not sure what impact that has on the PR's actually purpose)

This is only when the select param is used - see resources/views/layout.antlers.html. Remove the select param on 5.19 and the Actual is correct.

Logs

No response

Environment

Application Name: Statamic
Laravel Version: 11.19.0
PHP Version: 8.3.9
Composer Version: 2.7.7
Environment: local
Debug Mode: ENABLED
URL: bulk-augmentor.test
Maintenance Mode: OFF
Timezone: UTC
Locale: en

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: NOT CACHED

Drivers
Broadcasting: log
Cache: file
Database: sqlite
Logs: stack / single
Mail: log
Queue: sync
Session: file

Statamic
Addons: 0
Sites: 1
Stache Watcher: Enabled
Static Caching: Disabled
Version: 5.19.0 PRO

Installation

Fresh statamic/statamic site via CLI

Additional details

No response

[JohnathonKoster]

The entry_id is not being updated since it wasn't specified in the select list (the changes in #10226 restore the functionality of the select param, prior to 5.19, it just wasn't working).

The following will restore the behavior (but will defer to the Core team on if this is an actual bug, or not):

<ul>
    {{ nav:main select="title|url|id|entry_id" }}
    <li>{{ looper }}<br>{{ id }}</li>
    {{ /nav:main }}
</ul>

[martyf]

Thanks John, that makes sense and can confirm.

With that being the case, it would make more sense for the context to be a more accurate representation of what is actually available - not sure if that is possible with the way the internals work within Antlers. Just taking a step back to a bigger picture idea, if I select three properties, then realistically I should only see those selected properties, not properties that existed from an earlier iteration. Or even if scoping the selected variables would overcome that - but still would leave ambiguity as to which variable to use.

[JohnathonKoster]

not properties that existed from an earlier iteration

The values you receive are merged with the outer scope, and made available within the $context. The entry_id is not being cached between iterations when it's not in the select, but comes from the outer scope:

More specifically, it gets injected here: https://github.com/statamic/cms/blob/5.x/src/Structures/AugmentedPage.php#L40

If we change the return value, we can see that's the value that gets used:

Hope this helps a bit!

[martyf]

Yeah it makes sense what it is doing and not arguing with that at all, but from a DX perspective, it adds ambiguity given a variable is available when it wasn't part of the select request. In SQL terms, if I only request three columns with a select statement, that's all I get, and all I would expect.

In a perfect world, if I use a select param for three specific params, then I should only have access to those three params - which I do if I use the scope param (plus thinks like index, depth, etc, which are all great for that scope).

It would make more sense if that scope was almost there by default within a scoped block like a loop - and getting outer scope required an extra step, rather than the other way around. But I know that's a major thing, with a big flow-on effect for everyone's sites.

[JohnathonKoster]

I agree with what you're saying, and it's definitely possible. Maybe as a new param like select_only to not cause chaos for existing sites 🤔

Could also make something more general to restrict scopes in other places to "only" certain values without inheriting items from the outer scopes

[martyf]

That sounds awesome for sure!

I've been caught out with loops before for this reason - so try to use scope where possible (and in partials I prefix any partial-specific variables with an underscore to tell me that they're only intended in that file). Only seeing the selected fields within the loop would have made debugging this one easier: if my code failed because entry_id didn't exist it would have made more sense than showing the outer context's entry_id.