Pattern Parameters Not Always Working

[e2tha-e]

@bmuenzenmeyer Been wanting to contribute for a while, but was tied down with an insane deadline at work. Anyhow, the last version I had been using was 0.13.0, and after upgrading to 1.0.0, parameter submissions are broken.

To replicate:

• Edit source/_patterns/04-pages/00-homepage.mustache
• Replace with {{> templates-homepage(emergency: true) }}
• In 0.13.0, it works as expected, showing the emergency text.
• In 1.0.0 and the dev branch, everything above "Latest Posts" disappears.

I'm hoping to submit a pull request fairly soon in order to fix this, as well as making recursive parameter submission more robust.

[bmuenzenmeyer]

Good catch - I just confirmed this too. Frustrating when unit tests miss something, time for another one! It might be a good idea to add some parameters to the sample patterns as they are easy to spot check too.

If you don't get to the bottom of this I'll be looking into it soon too.

[bmuenzenmeyer]

The shipped organisms-sticky-comment pattern is working for me, however. Looking into the circumstances that distinguish this working example of parameters versus the broken one.

[bmuenzenmeyer]

Looks like 'patternlab.partials' object is empty at https://github.com/pattern-lab/patternlab-node/blob/dev/builder/parameter_hunter.js#L60

Seems like this slipped through other applications because of the nested nature of the template/pages reuse of the partial with pattern parameters, where other instances didn't need the partial object at all.

[geoffp]

@bmuenzenmeyer, I was just working on an underscore engine in the other branch, and I was wondering about the right way to do stuff like this.

From patternlab.js: What's the intended purpose of each of these?

patternlab.patterns = [];
patternlab.partials = {};

I know patternlab.patterns is meant to be array storage of every pattern template (do we call these partials?) that's been detected. patternlab.partials is less clear to me. Is it meant to be all patterns, plus "free" (non-pattern) partials from somewhere? Or is this meant to be the key/value version of patternlab.patterns?

[bmuenzenmeyer]

Hi @geoffp !

I know patternlab.patterns is meant to be array storage of every pattern template (do we call these partials?)

I call these patterns still, though they are of course standalone and can be made partials.

patternlab.partials is less clear to me. Is it meant to be all patterns, plus "free" (non-pattern) partials from somewhere? Or is this meant to be the key/value version of patternlab.patterns?

I've been thinking about this all night. Okay not like keep-me-up-at-night level stuff, but I'm beginning to think that the complete disuse of patternlab.partials is a vestigial organ of sorts from long dead development, replaced with the now useful getpatternbykey() abstraction. I missed this fact when coding the pattern parameter logic https://github.com/pattern-lab/patternlab-node/blob/dev/builder/parameter_hunter.js#L60
My first thought is to map patternlab.partials just in time for that call, but that's lazy.

What I'll probably do/propose is keeping patternlab.partials in sync with patternlab.patterns or replace it altogether. Gets ugly array indexing out of the solution but I am sure changing would have a cascading effect throughout the codebase. Having two nearly identical structures seems like codesmell.

[bmuenzenmeyer]

For those following along too - there needs to be a break; after https://github.com/pattern-lab/patternlab-node/blob/dev/builder/pattern_assembler.js#L179 but I haven't pushed that yet.

[geoffp]

I personally don't mind having two alternative data structure for holding patterns -- they'd just be storing references to the same objects, but ordered in different ways. Sometimes you may need the ordered, array-like pattern store; other times, you may need quick access by pattern key. Some smart person whose name I forget said something like, "if your data structures are correct, your algorithms will be trivial". I toyed with creating patternlab.patternsByKey = {...} for pattern-engines.

The interesting thing about the pattern-engines branch is that templating engines have different things they want around partials. Mustache wants you to hand it a big object of named partials to enable its native partial calls, though I'm not sure we use that in PL/Node because we recurse into partial calls and sort of "flatten" them manually so we can have stuff like style modifiers and parameters. Handlebars, on the other hand, wants you to call a function to register each partial so it can have its own, internal accounting of partials. To that end, I've added a registerPartial hook for engines that gets called by addPattern() if the engine implements it. Underscore is a nightmare because it doesn't even have a concept of partials at all, so you have to bolt one on (which my team has done).

[geoffp]

Thinking about it further, though -- I think getpatternbykey() is generally more flexible for fetching patterns, since it can support fuzzy matching, and a simple object store can't. But I do still need a big key/value blob to hand over to template engines. My vote therefore would probably be to maintain both data structures, and keep getpatternbykey(). I'm sure somebody will accuse us of bloat. :)

[bmuenzenmeyer]

This just landed in dev. Mustache requires that the third parameter, the partials object be a keyed with the partial name and then contain the template directly, so patternlb.partials is a simpler structure than patternlab.patterns[]

I think I missed a test scenario I will go back and check, but for the most part this is working

[bmuenzenmeyer]

This issue will close automatically when it reaches master