Unexpected MODX parser behavior change in 2.6

[Boddlnagg]

I have several issues on my site after an upgrade to 2.6, which I think are all caused by the same root issue. The main issue appears in my usage of the firstPageLink snippet, which does not work anymore. My code is as follows:

[[!Wayfinder? &startId=`0`
            &hereClass=`active`
            &firstClass=``
            &lastClass =``
            &excludeDocs =`5,6,20`
            &level=`1`
            &rowTpl=`@CODE <li [[+wf.classes]]><a href="[[!firstPageLink? &pid=`[[+wf.docid]]`]]" [[+wf.attributes]]>[[+wf.linktext]]</a>[[+wf.wrapper]]</li>`
        ]]

The problem is that [[+wf.docid]] which is passed to the pid parameter will not be evaluated before being passed to firstPageLink. And firstPageLink expects a page id, not the string "[[+wf.docid]]".

Another problem (also related to parsing order) is this code, which used to work, but does lead to an error now in 2.6:

[[[[!+modx.user.id:is=`3`:or:is=`10`:then=`!someSnippet`]]]]

I found this trick a few years ago in a forum and used it in some places, the idea being that [[!someSnippet]] should be called for users 3 and 10, but for other users, the snippet should not even be evaluated.

Is this expected change of behavior or is this a bug? I think it will cause major breakage also on other sites ... (on the other hand: great work that you did with 2.6, I really like the improvements there!)

[Boddlnagg]

Maybe this might be related to pdoTools? Because I do have pdoTools installed ...

[Boddlnagg]

More information: The problem seems to be somewhat different, and maybe the two issues are not that much related after all ...

Originally I had the Wayfinder snippet call cached ([[Wayfinder? ...]]), then I upgraded to 2.6.0, and found a message in the error log saying that cached snippet calls should not contain uncached snippet calls (because it contains the [[!firstPageLink ...]] call), so I changed it to uncached. On 2.6.0 it actually does not matter whether it is cached or uncached, it does not work anyway (i.e. [[+wf.docid]] does not get parsed anymore). After downgrading to MODX 2.5.8, I found out that it works only when Wayfinder is called cached, i.e. I can reproduce the problem on 2.5.8 when I call Wayfinder uncached. On 2.6.0, not even the cached version works anymore, so there is some change of behavior, but I'm not sure in what way.

The second problem that I mentioned is also still relevant, so I need to stay with 2.5.8 until this is fixed ...

[bezumkin]

Here is the main change of MODX parser.

Now you can`t call uncached tags inside cached, because it was wrong and leaded to bugs.

If you want to call snippets and output modifiers in chunks - use real chunks, not CODE or INLINE. Because snippets it them will be parsed before running the main snippet. I mean, firstPageLink will be called before Wayfinder, and WF will get already processed chunk.

Your second call must be rewritten as

[[![[+modx.user.id:is=`3`:or:is=`10`:then=`someSnippet`]]]]

It will be always uncached.

If you are already use pdoTools, you can use INLINE chunks with snippets and modifiers, but with another brackets {{!tag}}:

[[!pdoMenu?
    &parents=`0`
    &tpl=`@INLINE <li href="{{anySnippetThatWillBeParsedOnlyInChunk}}"></li>`
]]

pdoTools will replace them to [[!tag]] right at the processing time, not earlier.

So, now parser works more predictably. If you calling something inside uncached tag - you want to it run each time at page loads. So all inside this tag will be parsed before parent tag and parent tag will get already processed data to input. Always.

[Boddlnagg]

@bezumkin Thank you for this excellent explanation! I was able to fix both issues now with 2.6.0. (the second one required an additional ! for +modx.user.id, because the user ID must not be cached).

It might be a good idea to include a note in the changelog and/or release notes somewhere about this "breaking change". The way it's currently written in the changelog as Fix processing of noncacheable elements inside cached [#13530] does not make it clear that this breaks behavior that was "working" before (for some definition of "working").

[bezumkin]

(the second one required an additional ! for +modx.user.id, because the user ID must not be cached).

It can`t be cached, because it called inside uncached tag! See, how that simple now? =)

I`m wrong, see lower.

It might be a good idea

Maybe somebody must write an article about it

[Boddlnagg]

It can`t be cached, because it called inside uncached tag! See, how that simple now? =)

But it was cached! When I did not add !, then I could view the site as user 3 or 10 and see the result of the snippet call, and then view the site as guest and still see the result! So maybe there's still some bug?

[bezumkin]

Yes, you right, I`m wrong.

[[![[+modx.user.id:is=`1`:or:is=`10`:then=`Test`]]]]

In this case MODX parses this condition and saves result to cache.

We need to add additional token to user, so it can`t be cached

[[![[!+modx.user.id:is=`1`:or:is=`10`:then=`Test`]]]]

We really need an article about this logic!

[labr1005]

@bezumkin

In your PR you wrote:

People should not call uncached elements inside cached, so this fix will warn them about it in system log.

Which is somehow different to what you wrote here:

Now you can`t call uncached tags inside cached, because it was wrong and leaded to bugs.

Looks like I'll have to review and fix every such occurrency on every website that I want to upgrade to 2.6, right?

Or will the "wrong" occurrences still work and just warn me in the system log?

folds his hands

[bezumkin]

@labr1005

People should not call uncached elements inside cached Now you can`t call uncached tags inside cached

Okay, you can do it, but there is no point in this, so you should not. And, frankly, there is was no point in this earlier.

Or will the "wrong" occurrences still work and just warn me in the system log? Wrong without quotes.

Just think about cached tag, that contains any uncached parameter. What is the point of this cached tag, if any of its parameter can change at any time? This tag should be uncached, or you think different?

[Jako]

@bezumkin, @labr1005 Maybe the changed use cases should be specified a bit better:

As far as I understand, you could use uncached tags inside template chunks of cached snippet calls without a problem. I.e. in the tplWhatever chunk you could have uncached tags with the following snippet call.

[[Snippet? &tpl=`tplWhatever`]]

But there are some not really predictable use cases (which are logged with the change), that could have changed after #13250.

The following snippet call could produce a different output, depending on when the placeholder is set.

[[Snippet? &tpl=`@INLINE [[!+placeholder]]`]]

Maybe the description could be narrowed to: Don't use (directly visible) uncached tags in the properties of cached tags.

[bezumkin]

This will not trigger the error in log

[[Snippet? &tpl=`tplWhatever`]]

All tags in placeholder will be processed by the snippet, and if not - they will be leaved in document for later processing. For example, simple chunk for pdoResources:

[[!+pagetitle]] [[!+placeholder]]

will be saved in cache as I really don`t know why somebody need to leave unprocessed tags after snippet call, but you can, if you want.

But this will trigger error

[[Snippet? &tpl=`@INLINE [[!+placeholder]]`]]

and [[Snippet]] will be parsed as uncached at the end of document processing with all other uncached tags.

[2017-11-06 11:52:16] (ERROR @ /home/s11000/www/core/model/modx/modparser.class.php : 453) You should not call uncached elements inside cached!
Outer tag: [[pdoResources?
    &parents=`0`
    &tpl=`@INLINE [[!+pagetitle]] [[!+placeholder]]`
]]
Inner tag pdoResources?
    &parents=`0`
    &tpl=`@INLINE [[!+pagetitle]] [[!+placeholder]]`

And here what we see in document cache:

[Jako]

I really don`t know why somebody needed to leave unprocessed tags after snippet call, but you can, if you want.

Simple use case: A snippet creates a list and the generation is quite expensive, so the output should be cached. But now you need to output an active class or a selected attribute on runtime base. This could be done with an uncached placeholder, that is cached as a tag and will be replaced on runtime base outside of the snippet call, i.e.:

[[!+placeholder:eq=`[[+value]]`:then=`selected="selected"`]]

which will be parsed and cached during parsing the snippet call to:

[[!+placeholder:eq=`value`:then=`selected="selected"`]]

and will be parsed outside of the snippet call to selected="selected" if the placeholder is equal to value on runtime base.

[bezumkin]

Simple use case

No problem, this will still work without any errors.

and will be parsed outside of the snippet

And pdoResources with limit=10 will leave 10 unprocessed tags after work for later processing

[[!+placeholder:eq=`value`:then=`selected="selected"`]]
[[!+placeholder:eq=`anothervalue`:then=`selected="selected"`]]
...
[[!+placeholder:eq=`value10`:then=`selected="selected"`]]

[mrhaw]

It seems as this was the reason for the change: https://github.com/modxcms/revolution/issues/13250#issuecomment-313748558

goldsky provided a different PR: https://github.com/modxcms/revolution/pull/13531#issuecomment-313755040

I haven't researched this enough but posting for reference.

[matdave]

Thanks guys, I don't have much to add, but this helped me find some "inside out" parsing that was slowing down sites after upgrade to 2.6.0. HUGE thanks to debugParser tool as well.

[bezumkin]

Maybe we should close this issue? @Mark-H

[gpsietzema]

Looks to me this is all as designed. Good to still have this issue + solutions here for future reference.

@modxbot close