BUG: Fusion does not clear lastEvaluationStatus in certain case #3469

gjwnc · 2021-10-20T18:30:31Z

Description

We noticed, that in the Neos.Seo package, the rendered fusion output depends on the order of the evaluated properties which, I think, is based on a bug regarding an internal fusion state.

As an example, the following fusion code is relevant:

In, Neos.Seo:StructuredData.Website, the rendererd fusion output is different if you rearrange the properties to the following order:

    potentialAction = Neos.Seo:StructuredData.Website.SearchAction
    name = ${site.label}
    url = Neos.Neos:NodeUri {
        absolute = true
        node = ${site}
    }

If no targetNode is set and potentialAction is not the last one rendered, the output is correct.
On the other hand, if no targetNode is set and potentialAction is the last line evaluated, then @apply in Neos.Seo:StructuredData.Object is not evaluated correctly.

Which code steps are causing this behaviour

Assumptions and short overview:
Let's assume that targetNode is null. The property targetNode is used by the last rendered property potentialAction in Neos.Seo:StructuredData.Website.SearchAction. Thus the statement @if.hasTargetNode = ${this.targetNode} will be false.

What is happening:
In Neos.Seo:StructuredData.Object the @apply.attributes = ${props.attributes ? Array.filter(props.attributes, attribute => attribute != null) : []} will be evaluted incorrectly.
So let's start in evaluateApplyValues. In $key is the value attributes which is the correct fusion path to evaluate.
The @apply is then calling evaluate for all the attributes available, that is name, url and potentialAction. The last one evaluated is potentialAction.
For potentialAction, the method evaluateObjectOrRetrieveFromCache set's the local $evaluationStatus to EVALUATION_SKIPPED and then test's the @if condition using the targetNode which results in false, thus $evaluateObject = false.
Now the global lastEvaluationStatus is, kind of, correctly, set to skipped. Since this was the last evaluated statement, we get back to evaluateApplyValues. Since the internal lastEvaluationState is skipped, the @apply is completely ignored and $combinedApplyValues is empty - which it shouldn't in this example, since name and url were successful.

Steps to Reproduce

Install Neos.Seo package for demonstration purpose
Open the home of the site package
You should find something like this in <head>

<script type="application/ld+json">{"@context":"http:\/\/schema.org","@type":"WebSite"}</script>

This is wrong, since name (= site.label) and url should be in the json.
4. Rearrange the fusion properties of Neos.Seo:StructuredData.Website, such that potentialAction is not evaluated as the last one:

    potentialAction = Neos.Seo:StructuredData.Website.SearchAction
    name = ${site.label}
    url = Neos.Neos:NodeUri {
        absolute = true
        node = ${site}
    }

Now you get correct result in <head>

<script type="application/ld+json">{"@context":"http:\/\/schema.org","@type":"WebSite","name":"Some test site","url":"http:\/\/some.domain\/"}</script>

Expected behavior

The rendered fusion should be independent of the @if condition of the last evaluated property of a fusion prototype.

Actual behavior

@apply is not evaluated if the last evaluated property has false @if condition.

Affected Versions

Neos: >= 5.3 (very likely also earlier versions)

Flow:

The text was updated successfully, but these errors were encountered:

mhsdesign · 2022-03-09T19:06:53Z

Thanks for the report. I can confirm this.

Browserfied version: https://fusionpen.punkt.de/fusionpen/af3d466b1edcf85a44ae2866fd95db4bde668d9f.html

Fusion Code

prototype(Neos.Seo:StructuredData.Website) < prototype(Neos.Fusion:Component) {

    //                  //
    // Uncomment me!    //
    //                  //
    // potentialAction = Neos.Seo:StructuredData.Website.SearchAction
    name = ${site.label}
    url = Neos.Neos:NodeUri {
        absolute = true
        node = ${site}
    }
    //                  //
    // Comment me out!  //
    //                  //
    potentialAction = Neos.Seo:StructuredData.Website.SearchAction

    renderer = afx`<Neos.Seo:StructuredData.RootObject type="WebSite" attributes={props}/>`
}


prototype(Neos.Seo:StructuredData.RootObject) < prototype(Neos.Seo:StructuredData.Object) {
    context = 'http://schema.org/'
    @process.toJson = ${Json.stringify(value)}
    // @process.wrap = ${'<script type="application/ld+json">' + value +  '</script>'}
}


prototype(Neos.Seo:StructuredData.Object) < prototype(Neos.Fusion:Component) {
    // Optional context. Usually "http://schema.org" for root objects
    context = ''

    // The required `schema.org` type for this object
    type = ''

    // Inherit props from parent object into data. These can be accessed in the renderer via props.data.
    data = ${props}

    // All attributes will be applied to the resulting object
    attributes = Neos.Fusion:DataStructure

    renderer = Neos.Fusion:DataStructure {
        '@context' = ${props.context}
        '@context'.@if.isSet = ${props.context}
        '@type' = ${props.type}
        @apply.attributes = ${props.attributes ? Array.filter(props.attributes, attribute => attribute != null) : []}
    }
    @if.hasType = ${this.type}
}

prototype(Neos.Seo:StructuredData.Website.SearchAction) < prototype(Neos.Fusion:Component) {
    @if.hasTargetNode = ${this.targetNode}

    targetNode = null
    queryInput = 'required name=search_term_string'
    queryParameters = Neos.Fusion:DataStructure {
        search = '{search_term_string}'
    }

    renderer = afx`
        <Neos.Seo:StructuredData.Object type="SearchAction" attributes.query-input={props.queryInput}>
            <Neos.Neos:NodeUri absolute={true} node={props.targetNode} additionalParams={props.queryParameters}
                @path="attributes.target"
                @process.decode={String.rawUrlDecode(value)}/>
        </Neos.Seo:StructuredData.Object>
    `
}

mhsdesign · 2022-03-09T19:58:29Z

Shrinked down version to reproduce bug easily:

prototype(Neos.Apply.Bug:Demo) < prototype(Neos.Fusion:Component) {
    @process.htmlViewAble = ${Json.stringify(value)}

    //                  //
    // Uncomment me!    //
    //                  //
    // unevalObject = Neos.Fusion:Value {
    //     @if.never = false    
    // }
    
    normal = "things"
    foo = "bar"
    
    //                  //
    // Comment me out!  //
    //                  //
    unevalObject = Neos.Fusion:Value {
        @if.never = false    
    }

    renderer = Neos.Fusion:Component {

        explicit = "hello"
        attributes = ${props}

        renderer = Neos.Fusion:DataStructure {
            '@explicitPropsWork' = ${props.explicit}
            @apply.attributes = ${props.attributes ? Array.filter(props.attributes, attribute => attribute != null) : []}
        }
    }
}

mhsdesign · 2022-03-14T19:30:10Z

i think the problem lies somewhere in passing the lazy props and force the evaluation of those via an eel helper.

the props are here ${props.attributes ? Array.filter(props.attributes, attribute => attribute != null) : []} still lazy i think: eg not evaluated Array.filter will enforce the evaluation of the props ... and then something breaks.

when one leaves out the filter eg. only uses: @apply.attributes = ${props.attributes} it works...

mhsdesign · 2022-03-27T15:43:39Z

@gjwnc i looked at the runtime code now and can confirm its excacly like you said ;)

so lesson learned: never only use the $this->getLastEvaluationStatus() but also check if what you got is actually null.

…ionStatus()` fixes: neos#3469 doing the check if a render was successful cannot only happen via `$this->getLastEvaluationStatus()` -> as the render path might contain a lazy-prop (closure) which, when evaluated uses the same runtime. -> if the last lazy-prop has an `@if` annotation its skipped, but the `$lastEvaluationStatus` is set on the same runtime. -> there might still be partial successful output (the first values of a lazy-prop eg.) so we need to check additionally if the return value is null.

…ionStatus()` fixes: neos/neos-development-collection#3469 doing the check if a render was successful cannot only happen via `$this->getLastEvaluationStatus()` -> as the render path might contain a lazy-prop (closure) which, when evaluated uses the same runtime. -> if the last lazy-prop has an `@if` annotation its skipped, but the `$lastEvaluationStatus` is set on the same runtime. -> there might still be partial successful output (the first values of a lazy-prop eg.) so we need to check additionally if the return value is null.

mhsdesign mentioned this issue Mar 27, 2022

BUGFIX: Fusion Runtime lazy-props evaluation confuses `getLastEvaluat… #3677

Merged

kdambekalns closed this as completed in 1531182 Sep 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fusion does not clear lastEvaluationStatus in certain case #3469

BUG: Fusion does not clear lastEvaluationStatus in certain case #3469

gjwnc commented Oct 20, 2021 •

edited

mhsdesign commented Mar 9, 2022 •

edited

mhsdesign commented Mar 9, 2022 •

edited

mhsdesign commented Mar 14, 2022

mhsdesign commented Mar 27, 2022

BUG: Fusion does not clear lastEvaluationStatus in certain case #3469

BUG: Fusion does not clear lastEvaluationStatus in certain case #3469

Comments

gjwnc commented Oct 20, 2021 • edited

Description

Which code steps are causing this behaviour

Steps to Reproduce

Expected behavior

Actual behavior

Affected Versions

mhsdesign commented Mar 9, 2022 • edited

mhsdesign commented Mar 9, 2022 • edited

mhsdesign commented Mar 14, 2022

mhsdesign commented Mar 27, 2022

gjwnc commented Oct 20, 2021 •

edited

mhsdesign commented Mar 9, 2022 •

edited

mhsdesign commented Mar 9, 2022 •

edited