Pattern Data merge handling revisited

[vaxul]

Hi,

I ported a Node version to the latest PHP version of Pattern Lab duo to way faster pattern rendering speed.

There is only one major impact. The data handling from the JSON files seems to differ.

The _data/data.json define for example:

{
    "navigation": [
        {
            "level1Item": {
                "stuff": "abcd 1"
            }
        },
        {
            "level1Item": {
                "stuff": "abcd 2"
            }
        },
        {
            "level1Item": {
                "stuff": "abcd 3"
            }
        }
    ]
}

This results in every pattern, which uses "navigation", will show all 3 level1Items.

Now it is possible to disable the navigation for a specific pattern like "00-pattern.json":

{
    "navigation": false
}

Which works as expected.

But if a pseudo-pattern like "00-pattern~pseudo.json" re-declare "navigation" like:

{
    "navigation": [
        {
            "level1Item": {
                "stuff": "different 1"
            }
        },
        {
            "level1Item": {
                "stuff": "different 2"
            }
        }
    ]
}

... the navigation renders a merge from data.json and "00-pattern~pseudo.json" which results in someting like this:

{
    "navigation": [
        {
            "level1Item": {
                "stuff": "different 1"
            }
        },
        {
            "level1Item": {
                "stuff": "different 2"
            }
        },
        {
            "level1Item": {
                "stuff": "abcd 3"
            }
        }
    ]
}

I think the "00-pattern~pseudo.json" should build and merged on the latest "00-pattern.json"-data. This seem to be the case for the Node version.

Would this be even possible for the PHP version? It could be optional to prevent breaking.

I really would help, but I still something confused, where to look in the code.

Anyone with some hints?

[vaxul]

Got a bit deeper into the code (thanks xdebug).

Found out the final data for the rendering is get here: https://github.com/pattern-lab/patternlab-php-core/blob/46ae845673396ec67b24075d1ee66ac6ac10112d/src/PatternLab/PatternData/Helpers/PatternCodeHelper.php#L62

And is merged here: https://github.com/pattern-lab/patternlab-php-core/blob/46ae845673396ec67b24075d1ee66ac6ac10112d/src/PatternLab/Data.php#L310

What I'd like to do is to first merge global data with the "00-pattern.json" main JSON (where the pseudo is stems from) and afterwards merge "00-pattern~pseudo.json" which would result in the wished behavior. This should be optional.

[vaxul]

I found a solution.

It would be possible by extending following code https://github.com/pattern-lab/patternlab-php-core/blob/46ae845673396ec67b24075d1ee66ac6ac10112d/src/PatternLab/Data.php#L302-L338

... with this snippet within line 308:

// Different pattern data merge approach
$patternDataMergeVariant = Config::getOption('patternDataMergeVariant');
if ($patternDataMergeVariant == 2) {
    $originalPattern = PatternData::getPatternOption($patternPartial, 'original');

    // Only for pseudo patterns
    if($originalPattern) {
        // Reset global data with original pattern
        if (!empty($d["patternSpecific"][$originalPattern]["data"])) {
            $d = array_replace_recursive($d, $d["patternSpecific"][$originalPattern]["data"]);
        }
    }
}

For testing I created a semi-named new Pattern Lab config patternDataMergeVariant to be able to enable it optionally.

[vaxul]

Ah this would work for the specific case. But a lot of other pattern data seems to work different.

It's more like that a sub array is overridden by it's key. https://github.com/pattern-lab/patternlab-node/blob/31a5d639f8408200326cb708420f46c7cdceddb0/core/lib/patternlab.js#L375 Which uses lodash merge function.

I played around, but I currently don't have a clue how this would be possible in PHP.

EDIT: Ok, time for sleep. Of course it would necessary to use array_replace instead of array_replace_recursive... http://sandbox.onlinephpfunctions.com/code/9b8ae971b3619e8aec2803ad83d9c0ba1ee645fd

It might be a good idea to introduce an option to switch between the two behaviours with default to the current.

And the change in https://github.com/pattern-lab/patternlab-php-core/issues/160#issuecomment-364120250 would be another addition.

I would create a pull request. But here is still room for discussion. I would appreciate any opinions on this topic.

[vaxul]

Of course it would necessary to use array_replace instead of array_replace_recursive... http://sandbox.onlinephpfunctions.com/code/9b8ae971b3619e8aec2803ad83d9c0ba1ee645fd

It might be a good idea to introduce an option to switch between the two behaviours with default to the current.

After a some time testing I can conclude, that this doesn't work ether. The NodeJS merge version is just different.

I don't see another option then rework the data-JSONs.

[vaxul]

Closing because of no response.