patternlab-php-core icon indicating copy to clipboard operation
patternlab-php-core copied to clipboard

Global & Pattern Data Merge Issue

Open EvanLovely opened this issue 9 years ago • 8 comments

I'm seeing that if I have some global data that gets merged with pattern specific data that it is preserving far too much of the global data when it should of been overwritten by the local data.

It happens using the Data::getPatternSpecificData function and merges the data at PatternLab/Data.php:262 using array_replace_recursive().

I've broken it down to a simple example you can see here: https://repl.it/qOH/171

Is this intentional? Thoughts on fixes or workarounds @pattern-lab/drupal ?

EvanLovely avatar Apr 15 '16 00:04 EvanLovely

That's most definitely a bug. array_replace_recursive() doesn't go deep enough. That needs to be replaced by a custom function also found in the Data class. Blanking on the name. I must have missed that function call when I initially found the issue and created the fix. I know where it is I just haven't committed the fix. I should have it and a pattern state fix committed to dev in the morning.

dmolsen avatar Apr 15 '16 00:04 dmolsen

You rock Dave; thanks!

EvanLovely avatar Apr 15 '16 03:04 EvanLovely

Ok, this isn't quite as easy as I thought. I figured it'd be a quick replace but it turns out the deeper search and replace solution is just for link.. As such the required function is only accepting one argument when it needs to accepts two. Not a huge deal but it's more than I have time to address this morning. I'll look at fixing it this evening after soccer practice ;) At least I know the solution :)

dmolsen avatar Apr 15 '16 13:04 dmolsen

Thanks again! This would be great to get in as our current implementations put little to no stuff in data.json and heavily use the sidecar json (what I call the json files that have same name as pattern files). Isn't that what you're doing @aleksip ?

EvanLovely avatar Apr 15 '16 16:04 EvanLovely

Please double-check this is working the way you expect in v0.7.0. Thanks.

dmolsen avatar Apr 18 '16 14:04 dmolsen

It's still behaving the same way using v0.7.0 :(

EvanLovely avatar Apr 18 '16 18:04 EvanLovely

To demonstrate what I'm seeing, I've made a repo as an example. To setup:

git clone https://github.com/EvanLovely/test__pl-data-merge
cd test__pl-data-merge
php core/console --generate
php core/console --server
open http://localhost:8080

EvanLovely avatar Apr 19 '16 14:04 EvanLovely

I'm still thinking about this ;)

I may want to try to use a package, arraymerger, to provide a different way to merge data. This is on the radar to be tested before v3 of core.

dmolsen avatar Aug 05 '16 13:08 dmolsen