Handlebars.js: Support Dynamic Partial Blocks

[jstewmon]

Currently, there is no way of defining a dynamic partial with block syntax, so that a failover can be provided if the sub expression resolves to an undefined partial.

Why is this important?

When we use a dynamic partial block, the model-building code may not be aware of which partials are registered. For example, we may register partials with names that correspond to error codes, so that we can render an error-specific template in some cases, falling back to a default template. So, it would be convenient to supply a default template to use when the dynamic partial does not resolve to a registered partial.

I found that it is fairly trivial to support this behavior if the closeBlock helperName is optional:

diff --git a/lib/handlebars/compiler/helpers.js b/lib/handlebars/compiler/helpers.js
index 3c1a38f..f6295d8 100644
--- a/lib/handlebars/compiler/helpers.js
+++ b/lib/handlebars/compiler/helpers.js
@@ -1,9 +1,11 @@
 import Exception from '../exception';

 function validateClose(open, close) {
-  close = close.path ? close.path.original : close;
+  if (close.hasOwnProperty('path')) {
+    close = close.path ? close.path.original : close.path;
+  }

-  if (open.path.original !== close) {
+  if (close && open.path.original !== close) {
     let errorNode = {loc: open.path.loc};

     throw new Exception(open.path.original + " doesn't match " + close, errorNode);
diff --git a/spec/partials.js b/spec/partials.js
index 266837d..7982677 100644
--- a/spec/partials.js
+++ b/spec/partials.js
@@ -19,6 +19,20 @@ describe('partials', function() {
     shouldCompileToWithPartials(string, [hash, helpers, {dude: partial}], true, 'Dudes: Yehuda (http://yehuda) Alan (http://alan) ');
     shouldCompileToWithPartials(string, [hash, helpers, {dude: partial},, false], true, 'Dudes: Yehuda (http://yehuda) Alan (http://alan) ');
   });
+
+  it('dynamic partials with failover', function() {
+    var string = 'Dudes: {{#dudes}}{{#> (partial)}}Anonymous {{/}}{{/dudes}}';
+    var partial = '{{name}} ({{url}}) ';
+    var hash = {dudes: [{name: 'Yehuda', url: 'http://yehuda'}, {name: 'Alan', url: 'http://alan'}]};
+    var helpers = {
+      partial: function() {
+        return 'missing';
+      }
+    };
+    shouldCompileToWithPartials(string, [hash, helpers, {dude: partial}], true, 'Dudes: Anonymous Anonymous ');
+    shouldCompileToWithPartials(string, [hash, helpers, {dude: partial},, false], true, 'Dudes: Anonymous Anonymous ');
+  });
+
   it('failing dynamic partials', function() {
     var string = 'Dudes: {{#dudes}}{{> (partial)}}{{/dudes}}';
     var partial = '{{name}} ({{url}}) ';
diff --git a/src/handlebars.yy b/src/handlebars.yy
index ce06498..1df03cf 100644
--- a/src/handlebars.yy
+++ b/src/handlebars.yy
@@ -79,7 +79,7 @@ inverseChain
   ;

 closeBlock
-  : OPEN_ENDBLOCK helperName CLOSE -> {path: $2, strip: yy.stripFlags($1, $3)}
+  : OPEN_ENDBLOCK helperName? CLOSE -> {path: $2, strip: yy.stripFlags($1, $3)}
   ;

 mustache

Of course, this solution allows closing any block with {{/}}, which would be inadvisable as a regular practice, but doesn't implicitly harm anything. validateClose could include an additional check to only allow the empty close when open.type === 'SubExpression', so that only a dynamic partial block is allowed to have an empty close (I think that's the only case when the open will be of type SubExpression).

Another option is to allow partialName in the closeBlock:

diff --git a/spec/partials.js b/spec/partials.js
index 266837d..4800d3f 100644
--- a/spec/partials.js
+++ b/spec/partials.js
@@ -19,6 +19,20 @@ describe('partials', function() {
     shouldCompileToWithPartials(string, [hash, helpers, {dude: partial}], true, 'Dudes: Yehuda (http://yehuda) Alan (http://alan) ');
     shouldCompileToWithPartials(string, [hash, helpers, {dude: partial},, false], true, 'Dudes: Yehuda (http://yehuda) Alan (http://alan) ');
   });
+
+  it('dynamic partials with failover', function() {
+    var string = 'Dudes: {{#dudes}}{{#> (partial)}}Anonymous {{/(partial)}}{{/dudes}}';
+    var partial = '{{name}} ({{url}}) ';
+    var hash = {dudes: [{name: 'Yehuda', url: 'http://yehuda'}, {name: 'Alan', url: 'http://alan'}]};
+    var helpers = {
+      partial: function() {
+        return 'missing';
+      }
+    };
+    shouldCompileToWithPartials(string, [hash, helpers, {dude: partial}], true, 'Dudes: Anonymous Anonymous ');
+    shouldCompileToWithPartials(string, [hash, helpers, {dude: partial},, false], true, 'Dudes: Anonymous Anonymous ');
+  });
+
   it('failing dynamic partials', function() {
     var string = 'Dudes: {{#dudes}}{{> (partial)}}{{/dudes}}';
     var partial = '{{name}} ({{url}}) ';
diff --git a/src/handlebars.yy b/src/handlebars.yy
index ce06498..2d63945 100644
--- a/src/handlebars.yy
+++ b/src/handlebars.yy
@@ -79,7 +79,7 @@ inverseChain
   ;

 closeBlock
-  : OPEN_ENDBLOCK helperName CLOSE -> {path: $2, strip: yy.stripFlags($1, $3)}
+  : OPEN_ENDBLOCK partialName CLOSE -> {path: $2, strip: yy.stripFlags($1, $3)}
   ;

 mustache

I don't think this would be a very good solution in practice because dynamic partial expressions are rarely so trivial, so matching the sub expression in the openPartialBlock and closeBlock would be quite tedious.

I wanted to find a way to label an openPartialBlock containing a sub expression, but couldn't figure out a way of doing so that is clearly disambiguated from the existing partial syntaxes. I'd love to hear other syntactical suggestions, and I'd be happy to submit a PR if there consensus on a syntax.

[lawnsea]

From a syntax perspective, the decorator syntax that was introduced to support inline partials might be a good fit here. Something like this:

{{#*dynamic (partialName)}}
Fallback content
{{/dynamic}}

[nknapp]

Would be interesting to know if this can be implemented as decorator (assuming #1372 would be fixed).

Edit: No, my comment was dumb. It's not a decorator. If anything, it would be a helper, but helpers cannot easily call partials.
The syntax {{#*dynamic ...}} is already taken, if you do Handlebars.registeryDecorator('dynamic',...) before.

[nknapp]

EDIT: After thinking about this, the following is probably a very bad solution. I'm not sure about side-effects and it uses a lot of API that I wouldn't call public...

Something like this could be used to implement a default string for "all" missing partials.

const Handlebars = require('handlebars')

Handlebars.registerDecorator('onMissingPartial', function(program, props, container, decoratorContext) {
  const invokePartialWrapper = container.invokePartial
  container.invokePartial = function (partial, context, options) {
    if (props.partials[options.name]) {
      return invokePartialWrapper.apply(this, Array.prototype.slice.call(arguments))
    }
    return decoratorContext.fn(Object.assign({partialName: options.name, context}))
  }
});

var template = Handlebars.compile(`
{{#*inline "inlinePartial"}}
inlinePartial
{{/inline}}

{{#*onMissingPartial}}
Partial "{{partialName}}" could not be found
{{/onMissingPartial}}

{{>inlinePartial}}
{{>unknownPartial}}
`)

console.log("output", template({name: 'myPartial2'}))

It shows

Partial "unknownPartial" could not be found

It does not work with {{> (name) }}, but I think this may be a bug similar to #1372.

I'm trying to find a way without new language features, because we need a spec first (#1277), and at the moment, nobody seems to be writing one.

[nknapp]

I we would implement this, I think I'd go for the originally suggested {{/}} closing block.

[lawnsea]

The syntax {{#*dynamic ...}} is already taken, if you do Handlebars.registeryDecorator('dynamic',...) before.

Are you saying that there's code that already registers the dynamic decorator, or are you saying that code could register it?

[nknapp]

You could register it. But {{#*...}} means "apply decorator", so {{#* dyamic}} would mean "decorator" as well. {{#*inline}} is a decorator that adds a partial to the context. But calling a dynamic block-partial is not like a decorator.

Since {{helper}} becomes {{#block-helper}}...{{/block-helper}} for blocks and {{>partial}} becomes {{#>block-partial}}...{{/block-partial}}, and the {{> (dynamic-partial}} syntax already exists, I think that {{#> (dynamic-block) }} is the logical choice. The only missing thing is the end tag and {{/}} seems logical and does not break anything. (As far as I can see).

[rafegoldberg]

Failover content blocks for dynamic partials would be a _fantastic_ addition to Handlebars. Has there been any movement on this? Is there anything I can do to help push it along? (Also: much ♥ to any/all HBS collaborators for their fantastic work!)

[rafegoldberg]

Also, just came across an SO thread which correctly notes that you can use dynamic partials as block-level helpers _today_ with this ‘lil hack:

{{#>( lookup . 'intendedTemplate' )}}
  No template matched for "{{intendedTemplate}}"
 {{/undefined}}

Using {{/undefined}} as your closing tag feels hacky, to say the least. Still – even though it lacks the elegance of @nknapp's suggested syntax – it does indeed function as expected.

[jstewmon]

Sorry for bailing on this without a PR after all the constructive feedback. I got stuck trying to ensure the empty closing block wasn't allowable for other partial blocks, but I went back over everything today and think it's now working as intended in #1422.

[nknapp]

Thank you for the PR, @jstewmon. I think the final decision has to be made by @wycats because this is definitely a language enhancement and he asked me only to fix bugs. I'm not certain about his intended direction for the language.
If/when this principal issue is resolved, I have a couple of comments:

1) I would like to see one test with a template like {{#> (partial)}}failover content{{/}} where "partial" actually resolves to an existing partial.
2) How does the performance change with this commit?
3) If you want to see this feature in 4.1 (if @wycats approves), you should point your PR to the 4.x-branch. I'll take care of merging to master afterwards.

[nknapp]

Implemented in #1422 but not merged yet