WIP: Possible fix for #1252: Using @partial-block twice in a template not possible

[nknapp]

I've added a possible fix for this bug, but it includes some refactorings of the @partial-block-code. After thinking about it a lot (this is kind of a brain-twister), I've come to the conclusions that partial-blocks should be treated like function-closures:

With the example from one of the tests:

Template

<template>{{#> outer}}{{value}}{{/outer}}</template>

outer

<outer>{{#> nested}}<outer-block>{{> @partial-block}}</outer-block>{{/nested}}</outer>

inner

<nested>{{> @partial-block}}</nested>

Explanation

When the partial-block of the outer partial (<outer-block>{{> @partial-block}}</outer-block>) is executed, the data-variable @partial-block of outer must be called. The fix does not achieve this by popping the partial-block once its been used, but by storing the partial-block in the local closure-context of the invokePartial-function. The partial-block for that partial is than wrapped with another function that loads the previous partial-block from the local closure-context.

I can add more explanation, once my laptop-battery has recharged. In any way, I would like to have a review of the changes.

@lawnsea I changed some of the code from your fix for #1185, so could you review if my changes are valid in your eyes? The changes are in the branch "tmp/issue-1252".

After review, I would probably make some style changes for better readability.

[lawnsea]

I'm not sure this is something that the language should support. However, if we operate on the assumption that it should, this patch is what we'll need. However, I would like to see more test cases that explore how this feature would interoperate with nested partials and inline partials.

[lawnsea]

I'd like to see a few more test cases. I can think of at least two that we need:

• Multiple partial blocks in a nested partial block
  • Given these templates

  <outer>{{#> nested}}<outer-block>{{> @partial-block}} {{> @partial-block}}</outer-block>{{/nested}}</outer>

  <nested>{{> @partial-block}}</nested>

  <template>{{#> outer}}{{value}}{{/outer}}</template>

  • rendering with { value: 'success' } should produce

  <template><outer><nested><outer-block>success success</outer-block></nested></outer></template>
  

• Multiple partial blocks at different nesting levels
  • Given these templates

  <outer>{{#> nested}}<outer-block>{{> @partial-block}}</outer-block>{{/nested}}{{> @partial-block}}</outer>

  <nested>{{> @partial-block}}</nested>

  <template>{{#> outer}}{{value}}{{/outer}}</template>

  • rendering with { value: 'success' } should produce

  <template><outer><nested><outer-block>success</outer-block></nested>success</outer></template>
  

We probably also want to test that inline partials behave as expected when combined with multiple @partial-block invocations.

[nknapp]

I can add those tests.

[lawnsea]

So, previously, fn === partialBlock. Are you sure that we don't depend on that elsewhere?

[nknapp]

I have searched the lib-folder and the src-folder for occurences of the string partial-block in the tmp/issue-1252-branch and the only matches are in the functions invokePartial and resolvePartial. Those functions do not check for fn === partialBlock.

But no, I cannot be completely sure. I have just started digging in the internals of Handlebars, there might be something I don't see. I'm pretty confident though. Especially since all tests (including your new ones) are passing.

[lawnsea]

I am not sure the language should allow multiple @partial-blocks at a given nesting level. I'd like some feedback from the other maintainers and @wycats in particular.

[nknapp]

You also need this patch, if you want to use the #each-helper with partial-blocks

• Template: <template>{{#> list value}}value = {{.}}{{/list}}</template>
• Partial "list": <list>{{#each .}}<item>{{> @partial-block}}</item>{{/each}}</list>
• Input: { value: [ 'a', 'b', 'c' ] }
• Output: <template><list><item>value = a</item><item>value = b</item><item>value = c</item></list></template>

I think this should work, because it already worked in 4.0.3 and 4.0.4, but I agree that we should wait for feedback from @wycats and others.

[nknapp]

@wycats The question we are having here is:

Should this be possible: <partial>{{@partial-block}} - {{@partial-block}}</partial>
and this: <partial>{{#each list}}{{@partial-block}}{{/each}}</partial>

It was possible in version 4.0.3 and 4.0.4, but there were other problems (#1185). Versions 4.0.5 and 4.0.6 do not allow it.

You said you don't want new features to be implemented until we have a spec. Do you consider the above examples to be a new feature or a fix?
My opinion is that it is a fix. Since a @partial-block is basically the equivalent of the options.fn()-callback of a block-helper, it should work like options.fn(), even if it is called multiple times.

@lawnsea I have added multiple test-cases for your scenarios and a few more (e.g. a partial-block called from within an #each block. Those tests pass.

[lawnsea]

@nknapp I've come around on this, particularly since it used to work and doesn't anymore, thanks to me. I still think <partial>{{@partial-block}} - {{@partial-block}}</partial> is weird and not very compelling, but <partial>{{#each list}}{{@partial-block}}{{/each}}</partial> is pretty clearly useful.

Thanks for adding the test cases. I'll take a look.

[lawnsea]

Let's give @wycats a couple more days to weigh in. If we don't hear from him, I think we should land this and cut a 4.0.7 release.

[nknapp]

This has been waiting for almost a month now. I have not had the time to do it, but I will merge it now.

[nknapp]

I have condensed the branch into 2 commits

• The fix with a test for the fix
• Additional testcases for partial-blocks