Partial with {{> @partial-block }} should not be rendered when processed as stand-alone templates

[teehemkay]

I am using Pattern Lab Node edition-node-gulp@1.3.2 with patternengine-node-handlebars@1.0.0

Expected Behavior

Let's suppose we have the following two templates:

Template foo.hbs:

{#> bar }}
  Foo
{{/bar}}

Template bar.hbs:

{{> @partial-block }}
Bar

I would expect this to render as:

Foo
Bar

Actual Behavior

Instead I get the following error message:

[14:01:52] Error: The partial @partial-block could not be found
    at Object.invokePartial (/Users/[...]/test-case/node_modules/handlebars/dist/cjs/handlebars/runtime.js:266:11)
    at Object.invokePartialWrapper [as invokePartial] (/Users/[...]/test-case/node_modules/handlebars/dist/cjs/handlebars/runtime.js:68:39)
    at Object.eval (eval at createFunctionContext (/Users/[...]/test-case/node_modules/handlebars/dist/cjs/handlebars/compiler/javascript-compiler.js:254:23), <anonymous>:6:28)
    at main (/Users/[...]/test-case/node_modules/handlebars/dist/cjs/handlebars/runtime.js:173:32)
    at ret (/Users/[...]/test-case/node_modules/handlebars/dist/cjs/handlebars/runtime.js:176:12)
    at ret (/Users/[...]/test-case/node_modules/handlebars/dist/cjs/handlebars/compiler/compiler.js:525:21)
    at Object.renderPattern (/Users/[...]/test-case/node_modules/patternengine-node-handlebars/lib/engine_handlebars.js:44:12)
    at Object.render (/Users/[...]/test-case/node_modules/patternlab-node/core/lib/object_factory.js:76:26)
    at renderPattern (/Users/[...]/test-case/node_modules/patternlab-node/core/lib/pattern_assembler.js:143:22)
    at Object.renderPattern (/Users/[...]/test-case/node_modules/patternlab-node/core/lib/pattern_assembler.js:536:14)

Steps to Reproduce

[geoffp]

Thanks for the report, @teehemkay!

[teehemkay]

What I think is happening is that Handlebars is being fed a template A that includes a {{> @partial-block }} expression as a "stand-alone" template: meaning not as a template being included as a partial from another template B.

In such a context Handlebars runtime seems to interpret the {{> @partial-block }} expression as a normal include of a partial named "@partial-block" which it can't find of course.

In "normal" Handlebars use case such a situation would typically be an error since template A doesn't make much sense as a "stand-alone" template.

But PL, tries to render every template. And there in lies the problem.

So my current understanding is that {{> @partial-block }} hbs expression should probably NOT be processed when their enclosing template is invoked as a "stand-alone" template.

Also I've looked at the code of the hbs engine and it seems to be doing the right thing by not recognizing @partial-block as the name of a partial.

I have tried excluding the template by listing its containing directory in the styleGuideExcludes property of my patternlab-config.json. My understanding is that this should prevent the template from being built but that doesn't seem to be the case.

[teehemkay]

@geoffp On Gitter, @bmuenzenmeyer was kind enough to provide some pointers to the main modules involved in how the engines are hooked up to the core PL.

This has enabled me to modify the hbs engine as follows to correct this issue in basically 3 steps:

First add a findAtPartialBlockRE to engine_handlebars with the following value:

findAtPartialBlockRE: /{{#?>\s*@partial-block\s*}}/g`

Second add an escapeAtPartialBlock property to engine_handlebars defined as follows:

function(partialString) {
  var partial = partialString.replace(this.findAtPartialBlockRE, '{{> @partial-block }}')
  return partial;
}

And third add the following line to in renderPattern before compiling the template:

pattern.extendedTemplate = this.escapeAtPartialBlock(pattern.extendedTemplate);

This solved the issue for me.
I'm not sure this is the appropriate way to do it (e.g. may this is internal machinery that shouldn't be exposed in the engine API?) but I hope this helps anyway.

@geoffp Please let me know if you want me to create a pull request

[geoffp]

@teehemkay excellent work. I would love a pull request, with three requests:

1. Put escapeAtPartialBlock() out in the module scope, since you're right, it doesn't need to exposed as API. While you're at it, move those regexes out to the module scope, and declare them there as const. We don't need to expose those as API, either!
2. Try calling escapeAtPartialBlock() in registerPartial() instead. That should get the escaping done earlier in the build process, and seems a bit safer.
3. (bonus) For mega bonus points, if you want to get your hands dirty, write a test case for this in the handlebars tests in the main patternlab-node repo that fails until you've got your fix applied to the handlebars engine. If you're not familiar with npm link, I highly recommend it for this kind of thing.

I'll be out of town for a week, so no rush.

[geoffp]

Also, I think this is my old, personal repo for patternengine-node-handlebars -- I think we should be doing this at https://github.com/pattern-lab/patternengine-node-handlebars instead.

Just noticed that!

[teehemkay]

@geoffp Cool. I'll create the PRs later this week.

[teehemkay]

@geoffp re: point 2. above: it doesn't work to call escapeAtPartialBlock() in registerPartial().

The escaping must only occur when a partial with a {{> @partial-block }} expression is rendered as a standalone template. Escaping too early such as in registerPartial() would prevent the inclusion mechanism from working when it's needed...

[geoffp]

@teehemkay -- good find. Thanks for trying it. I'll check out the PR.

[bmuenzenmeyer]

@teehemkay @geoffp

Let me know if / when pattern-lab/patternlab-node#548 can be merged. Sounds like this is still baking?

[teehemkay]

@geoffp my PR @ pattern-lab/patternlab-node#548 contains tests for this PR. One of them will keep failing as long as this PR as not been accepted...

[geoffp]

Cool. I'll review and hopefully merge ASAP.