Revolution: parseChunk does not give precedence to values passed in

[pixelchutes]

Summary

When passing an associative array to $modx->parseChunk(), the values parsed in are not given priority processing order, and therefore are not always honored if you have a conflicting, preexisting placeholder set before it.

Step to reproduce

1. Create a Plugin (OnWebPageInit) that sets a placeholder on each request:

   $modx->setPlaceholder('example', 'ABC' );
   

2. Create a Chunk (example) that outputs the placeholder:

   Example: [[+example]]
   

3. Create and run a Snippet, invoking parseChunk, but with custom values:

   return $modx->parseChunk( 'example', array( 'example' => 'XYZ' ) );
   

   Observed behavior

Output = Example: ABC

The parseChunk method calls getChunk resulting in the conflicting placeholder being merged _first_, prior to parsing the array values provided.

Expected behavior

Output = Example: XYZ

Environment

MODX 2.3.4-dev

[OptimusCrime]

Not 100% sure how to go about this, as it seem like a error caused by the underlying implementation. As you have observed, it happens because you call getChunk before applying the placeholders.

However, at the same time, the description for the method says the following:

parseChunk simply does a str_replace on the values in the associative array you pass it, so you cannot use Output Filters or other complex placeholder modifiers here. If you require more functionality from the parser, see modX.getChunk instead.

While getChunk:

getChunk will execute the MODX parser, so you can use output filters and the $properties array can be multi-dimenstional (i.e. more than just simple key/value pairs). If this is more horsepower than you need, use modX.parseChunk instead.

I think this is pretty misguiding, as it turns out both methods invokes the MODX parser.

I think one possible way to solve this could be:

$properties = array();
if ($prefix === '[[+' and $suffix === ']]') {
    $properties = $chunkArr;
}

$chunk = $this->getChunk($chunkName, $properties);

However, this seem pretty dumb as well, as we are now simply reusing the logic from getChunk.

Another option would be to avoid invoking the parser all together. I made the following code work like you expect:

$chunk = '';
if ($this->getParser()) {
    $chunkInstance = $this->parser->getElement('modChunk', $chunkName);
    if ($chunkInstance instanceof modChunk) {
        $chunk = $chunkInstance->get('content');
    }
}

Then again, this would most likely be a HUGE breaking change for anyone that uses parseChunk.

I'm leaning towards closing this as an intended feature of the method.

[pixelchutes]

@OptimusCrime technically I believe the handling could be considered regression (maybe?) at least when compared to how Evo parseChunk worked.

IIRC Jason confirmed it as a bug, but he also intended to officially deprecate this method anyway. Not sure when that will happen, but I'm fine closing it as "won't fix."

Nevertheless, thanks for digging in and looking for a possible solution!

[OptimusCrime]

No problem!

[pbowyer]

With 3.0 coming soon, this would be a good time to revisit this ticket and deprecate parseChunk().

[OptimusCrime]

@pbowyer What would be the motivation for deprecating parseChunk? It does what it says it does. It is a super simple version of getChunk.

[pbowyer]

@OptimusCrime The comment above says:

IIRC Jason confirmed it as a bug, but he also intended to officially deprecate this method anyway. Not sure when that will happen, but I'm fine closing it as "won't fix."