Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement partial blocks #1076

Merged
merged 4 commits into from
Aug 24, 2015
Merged

Implement partial blocks #1076

merged 4 commits into from
Aug 24, 2015

Conversation

kpdecker
Copy link
Collaborator

This allows for failover for missing partials as well as limited templating ability through the {{> @partial-block }} partial special case.

Partial fix for #1018

kpdecker added a commit to wycats/handlebars-site that referenced this pull request Aug 14, 2015
@kpdecker
Copy link
Collaborator Author

@mmun @wycats could I get your input on this? Trying to carve off pieces from #1018 that can go out in a more standalone fashion vs. a big bang meta feature.

My biggest questions here are:

  1. Does the feature generally make sense in isolation?
  2. Is the naming of @partial-block sensible?
  3. Do we need a separate AST node for this or should we merge the two and use the existence of program as the differentiator in behavior? This is effectively the way that it's implemented now but it has all of the extra overhead of handling the distinct AST type.

Documentation PR here: wycats/handlebars-site#120

kpdecker added a commit to wycats/handlebars-site that referenced this pull request Aug 14, 2015
@mmun
Copy link
Contributor

mmun commented Aug 16, 2015

Ember doesn't support {{> }} constructs so I don't have a lot of experience.

  1. Roughly, yes.
  2. It's ok, but I think a better solution is to introduce a {{yield}} + an isolated "partial" AKA components.
  3. In this case, having a single AST node seems better if you can get away with it. (With mustache/block you can't because mustaches have an escaped field).

@kpdecker
Copy link
Collaborator Author

  1. Ok
  2. I'm not sure what you are suggesting. Could you provide a code example?
  3. I was hoping that this could be done, but strip behavior was sufficiently different and there is also something to be said for introducing new types so non-supporting code doesn't have surprises, i.e. the program could be completely ignored, silently, vs. a thrown error.

@mmun
Copy link
Contributor

mmun commented Aug 19, 2015

Since the whitespace constraints are different I don't have anything useful to add. Is @partial-block implemented via "data" or is it effectively a keyword?

FWIW, I am still interested in the original named blocks proposal.

EDIT: Ah the numbering got messed up on your comment (Thanks markdown!) I'll comment again shortly.

@mmun
Copy link
Contributor

mmun commented Aug 19, 2015

In ember you can write something like

{{#each-with-ul items=people as |p|}}
  Hello {{p.name}}
{{/each-with-ul}}

where the each-with-ul component is defined in a separate file as

<ul>
  {{#each items as |item|}}
    <li>{{yield item}}</li>
  {{/each}}
</ul>

This generates the output (ignoring whitespace)

<ul>
  <li>Hello Kevin</li>
  <li>Hello Yehuda</li>
</ul>

I assume Thorax has something similar. The win here is that the scope of the second template above is completely encapsulated so its easier to reason about and easier to reuse. @partial-block is similar to yield, but with the partial context "leaking" behaviour.

Aligning Handlebars with yield would also give named blocks a better chance of landing, as they don't make much sense if you can't yield to them.

This allows for failover for missing partials as well as limited templating ability through the `{{> @partial-block }}` partial special case.

Partial fix for #1018
Avoid duplicating the logic needed to check for close block mismatches.
@kpdecker
Copy link
Collaborator Author

I'm having trouble working through your comments about encapsulation and leaking contexts or how this is different from the helper block-based ember example above. Can you help me understand that?

Regarding named blocks, the decorators branch implements a generic way to apply metadata to blocks and wrap in behaviors as well as implements the inline partials behavior that is the end goal. This PR needs to land before can do too much discussion on that, unfortunately.

@mmun
Copy link
Contributor

mmun commented Aug 22, 2015

Forget it, the docs give an example that removes my concerns.

{{#each children as |child|}}
  {{#> childEntry}}
    {{child.value}}
  {{/childEntry}}
{{/each}}

This works as expected, even if your childEntry partial was something like

{{#with somethingElse as |child|}}
  {{> %block}}
{{/with}}

it wouldn't cause any shadowing of child.

@kpdecker kpdecker mentioned this pull request Aug 22, 2015
kpdecker added a commit that referenced this pull request Aug 24, 2015
@kpdecker kpdecker merged commit c727e1f into master Aug 24, 2015
@kpdecker
Copy link
Collaborator Author

Sounds like we're mostly in agreement then. Leaving the token name as @partial-block and merging as is to unblock the decorators PR and the release in general.

@kpdecker
Copy link
Collaborator Author

kpdecker commented Sep 1, 2015

Released in 4.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants