When a pattern is included, it inheriting data from data.json instead of the sibling json file

[acarnwath]

I am using Pattern Lab Node v1.2.2 on Mac, using the gulp configuration.

Expected Behavior

When a pattern is included, it should inherit from the following in order:

1. The includer's (similarly named) json file
2. The included pattern's (similarly named) json file -- supplementing what does not already exist from 1
3. data.json -- supplementing what does not already exist from 1 or 2

We are creating an enterprise-level site with a lot of pattern reuse and inclusion in other patterns. A lot of the data is shared, but requires tweaks to certain attributes. This has caused a huge amount of bloat in data.json, in order to allow for the inheritance of shared data.

Actual Behavior

When a pattern is included, it inherits from:

1. The includer's (similarly named) json file
2. data.json -- supplementing what does not already exist from 1

Steps to Reproduce

Mustaches & JSON

_data/data.json

{
    "test": {
        "title": "data.json Title",
        "data": "data.json Data"
    },
}

_patterns/00-atoms/test.mustache

<p>
    Title: {{test.title}}<br />
    Data: {{test.data}}
</p>

_patterns/00-atoms/test.json

{
    "test": {
        "title": "test.json Title",
        "data": "test.json Data"
    }
}

_patterns/01-molecules/test-molecule.mustache

<p>This is a molecule that contains test</p>
<p>Molecule Data: {{molecule-test}}</p>
{{> atoms-test }}

_patterns/01-molecules/test-molecule.json

{
    "molecule-test": "Molecule Test Content"
}

_patterns/01-molecules/test-molecule~with-title.json

{
    "test": {
        "title": "test-molecule~with-title.json Title"
    }
}

Result

?p=atoms-test: Actual & Expected

<p>
    Title: test.json Title<br>
    Data: test.json Data
</p>

?p=molecules-test-molecule: Actual

<p>This is a molecule that contains test</p>
<p>Molecule Data: Molecule Test Content</p>
<p>
    Title: data.json Title<br>
    Data: data.json Data
</p>

?p=molecules-test-molecule-with-title: Actual

<p>This is a molecule that contains test</p>
<p>Molecule Data: Molecule Test Content</p>
<p>
    Title: test-molecule~with-title.json Title<br>
    Data: data.json Data
</p>

?p=molecules-test-molecule: Expected

<p>This is a molecule that contains test</p>
<p>Molecule Data: Molecule Test Content</p>
<p>
    Title: test.json Title<br>
    Data: test.json Data
</p>

?p=molecules-test-molecule-with-title: Expected

<p>This is a molecule that contains test</p>
<p>Molecule Data: Molecule Test Content</p>
<p>
    Title: test-molecule~with-title.json Title<br>
    Data: test.json Data
</p>

[bmuenzenmeyer]

Hey @acarnwath

Thanks so much for the thorough report! I'll make sure to take a look at this soon

[JennaPG]

YES! THIS ^^. Our data.json file is massive and super annoying. Would love to use data partials but the fact they don't work when you pull patterns into templates (or other patterns) makes them useless, and we're stuck with throwing everything into data.json. This would be an awesome improvement.

[e2tha-e]

@JennaPG You might want to try this tool which extends the abilities of Pattern Lab to do what you wish: https://github.com/electric-eloquence/fepper#configuration

In the readme, it states you can use underscore-prefixed .json files in the patternlab-node/source/_patterns to act as partials to the final global data.json file.

As for the original issue described by @acarnwath , Fepper doesn't tackle that. As for whether Pattern Lab Node will, I think depends on how severely it affects the already expensive recursion (for pattern inclusion alone) along with even more expense to read the json for included patterns, and then merge that data with the global data, and then only for the HTML of the block of the included pattern.

@acarnwath I'm pretty sure I've tested this myself in Pattern Lab PHP, and I think the issue you describe affects that as well. But please let us know if I'm wrong.

[bmuenzenmeyer]

Hi all

Thanks for the input. I completely understand the use case and can identify with its need. When the partial-expanding algorithm was made it did not account for the desire to integrate partial.json files along the way, instead assuming the user would override anything they wish to at the top-most level. If what I am hearing is correct, this is simply a wrong, unsustainable omission, one that I have yet to encounter in my own use of the project and certainly not revealed with the shipped example patterns.

I appreciate people using PL Node in big ways and stretching it to its limits - perspectives other than my own have and will continue to shape development. I want to prioritize this - but am in the middle of a large undertaking as well as up against the impending birth of our second son. That might be a net-win for free time, but then again, maybe not! 🍼

Stay tuned, or if someone wants to take a stab at it - I think it'd be best targeted to the dev-2.0-core branch.

[james-nash]

FWIW, I'm pretty sure PHP PatternLab has the same behaviour (i.e. ignores the partial's sibling data JSON and goes right back to data.json), so I believe Node PatternLab's current behaviour is consistent with that.

That being said, I would prefer the requested behaviour (and ditto for the PHP version). In the interest of being compatible with the PHP version, perhaps both should be updated. Or maybe, there could be a config option to toggle between the two behaviours. That way, patterns that rely on the existing behaviour don't break, but people have the choice of using the, IMHO more useful, "use partial's data.json" behaviour.

[e2tha-e]

@bmuenzenmeyer Super congrats on the little one! (Maybe I'm a little early on that, but you know what I mean!)

As food for thought on this issue, one might want to consider that templating languages in and of themselves do not do this ask. Using Handlebars as an example, it extends Mustache to (among many other things) accept parameters within its partials. However, these parameters need to come from upstream, from the data for the original parent pattern. It does not provide a way to be aware of a context specific and unique to included partials.

These templating languages, like Pattern Lab, need to be mindful of their performance and their code bloat, since they can and do get scaled to precarious heights. I know for the projects I work on using these templating languages (pretty much everything I work on for pay), we manage to get by without the functionality requested in this issue. We can find ways to work around this shortcoming (and it is a legitimate shortcoming.)

[dmolsen]

@acarnwath -

Because this should be implemented across both Pattern Lab projects @bmuenzenmeyer & @geoffp asked me to chime in. After thinking about it and sketching a possible implementation I'm ok with implementing this as long as it has an opt-in flag as noted by @james-nash. Between perf and breaking a very specific expectation I'd like people to have the option of deciding for themselves if they'd like to take advantage of it. The perf aspect makes me a little queasy but I'm willing to at least see how it goes as I'm not sure it's a very big change from a code perspective in PL2 PHP. I also like that it makes pseudo-patterns more useful.

The keys for implementing this feature seem to be have the lineage data stored and then building/storing the fully merged data only once for each pattern using the lineage so it's only a couple of looks up for data rather than deep recursion and duplicative work for each pattern. This will also ensure that we don't have to muck with pattern engines at all. That probably doesn't make sense here but it makes sense when I think about my code ;)

I don't have a timeline for implementation. I have some front-end JavaScript I need to finalize before coming back to any many back-end features.

[bmuenzenmeyer]

This could now be accomplished via plugin

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.