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

Yieldable named blocks #460

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@wycats
Copy link
Member

wycats commented Mar 7, 2019

Rendered

@wycats wycats force-pushed the wycats:yieldable-named-blocks branch from 153ae6a to 2f4c4e2 Mar 7, 2019

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Mar 7, 2019

@wycats it is very exciting to see this feature moving forward, thank you for working on it!

I noticed that this RFC specifically does not make any changes to yield, which leaves one of the nice features in the named block RFC out: the concept of unified renderables. While I definitely understand the desire to cut scope in order to actually ship something, I think that this RFC should at least mention that the specific concept is either rejected completely or is ripe for a future RFC.

Specifically, as a component author I want to allow a caller to pass any number of things (plain string, closure component, or a named block). In order to do that (unless I've misunderstood things in this RFC) is via something akin to the following snippet in the component's template:

<article>
  <header>
    {{#if (has-block 'title')}}
      {{yield to="title"}}
    {{else}}
      {{@title}}
    {{/if}}
  </header>
  <section>
    {{if (has-block 'body')}}
      {{yield to='body'}}
    {{else}}
      {{yield}}
    {{/if}}
  </section>
</article>

This allows the callsite to use any of the following invocations:

<Article @title={{@article.title}}>
  {{! block contents here }}
</Article>
<Article @title={{component 'special-article-title'}}>
  {{! block contents here }}
</Article>
<Article>
  <:title><b>{{@article.title}}</b></:title>
  <:body>
    {{! block contents here }}
  </:body>
</Article>

While thats totally fine its a bit gnarly to type out each time, and since blocks themselves can't be passed (mentioned in the RFC as capturing) it can't be abstracted away into a shared control flow component.

What is the plan for this? Just keep the verbosity here, and come up with some clever AST "macros"?

I'm also curious how the longer term plan to be able to pass these blocks around would work, have you thought much about that? Since it seems that this design intentionally does not limit named blocks to exist in the same conceptual space as named arguments, naming conflicts are likely (see API I demo'ed just above :P ), that seems vaguely concerning unless we have a relatively good idea how we plan to reference a passed in block (and that that isn't as a named argument).

@wycats

This comment has been minimized.

Copy link
Member Author

wycats commented Mar 7, 2019

@rwjblue The concept of "unified renderables" as envisioned in #226 was described (and carefully specified) in more detail in #432, so I left it out of this RFC.

From the perspective of this RFC, if you have a component that takes a block for title and you want to pass a string, you would still use the block syntax to pass it:

<Article>
  <:title>{{@article.title}}</:title>
  <:body>
    {{! block contents here }}
  </:body>
</Article>

It's slightly more verbose than @title=, but only slightly, and it maintains a consistent mental model for what you're passing along.

It also makes upgrading from a simple block to a block that takes named arguments easier to teach, implement (and use), which was a thorny aspect of the original proposal.

That said, when we do decide on a strategy for reifying blocks in a future RFC, we will have to decide what happens when you put one inside of {{...}}. I can easily believe that we would make that work, and I think it's a good topic of discussion for that RFC.

@wycats

This comment has been minimized.

Copy link
Member Author

wycats commented Mar 7, 2019

I'm also curious how the longer term plan to be able to pass these blocks around would work, have you thought much about that? Since it seems that this design intentionally does not limit named blocks to exist in the same conceptual space as named arguments, naming conflicts are likely (see API I demo'ed just above :P ), that seems vaguely concerning unless we have a relatively good idea how we plan to reference a passed in block (and that that isn't as a named argument).

My thinking is that we'd use :foo (not inside of a string) as the name for reification.

<Article :title=:headline />

In this example, the :headline block in the current template would be passed to Article as the :title block. This is purely speculative, but it's an example of how we could maintain the namespace separation going forward.

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Mar 8, 2019

It also makes upgrading from a simple block to a block that takes named arguments easier to teach, implement (and use), which was a thorny aspect of the original proposal.

Sure, I see what you mean here but until the broader ecosystem can assume newer Ember versions (which support this new named block syntax) addon components would have to continue to use the snippet I demoed above (which kinda sucks). It would be nice to have something nicer for them to use, but again maybe we could do it via an AST transform ala a macro.

@amyrlam amyrlam referenced this pull request Mar 8, 2019

Merged

Ember Times No. 88 - March 8, 2019 #3849

5 of 16 tasks complete
@pzuraq

This comment has been minimized.

Copy link
Contributor

pzuraq commented Mar 8, 2019

Really like this proposal overall! I think this is well-scoped, and minimal, without blocking us from iterating on it in the future. Most of my concerns are for future functionality actually, I just want to note them down here so they're available when we get around to it:

  • I like the idea of being able to assign blocks using :foo=, but I don't think it makes much sense to be able to refer to blocks within a template with :foo on the right hand side. If we compare to named arguments, it's inconsistent:

    <!-- @bar refers to the argument -->
    <div @foo={{@bar}}></div>
    
    <!-- @bar refers to the argument, and `<>` invokes the argument as a renderable -->
    <@bar></@bar>
    
    <!-- :bar refers to the argument -->
    <div :foo=:bar></div>
    
    <!-- :bar captures a block -->
    <:bar></:bar>

    I think it could get very confusing when sometimes the identifier is used to refer to the block value, and sometimes it is used to capture/assign the value. Instead, I think we could use the yield helper, and allow it to be used inline:

    <div @foo={{yield to="bar"}}></div>

    This means that invocation and capturing are always consistent.

  • In designing Ember Table, one use case that came up consistently was wanting named blocks, and wanting them to be able to receive their own arguments:

    <EmberTable as |t|>
      <t.head @columns={{columns}} />
    
      <t.body @rows={{rows}} />
    </EmberTable>

    I think that this will also be something we want if/when we get to block capture, but it's definitely outside of the scope of this RFC. It's also something that other frameworks can accomplish today, and I think would be very convenient.

    One possibility would be if blocks were captured on something like this.blocks, then we could refer to block arguments via this.blocks.NAME.args:

    <EmberTable>
      <:head @columns={{columns}}/>
      <:body @rows={{rows}}/>
    </EmberTable>
    this.blocks.head.args.columns;
    this.blocks.body.args.rows;

    It's a little verbose, but not too bad - I'd personally be ok with that style of access. I just want to make sure we don't foreclose on the possibility of adding block arguments down the road, since like I said, I think they'd be very useful (along with generic block capturing)

@rtablada

This comment has been minimized.

Copy link

rtablada commented Mar 8, 2019

@wycats really exciting to see this feature moving forward and coming to fruition.

While after reading for a bit I was able to parse the example in the "block parameters" section it was a bit unclear what was the actual result. On first glance it looked like the positional params were fallbacks if the block was not invoked.

It took some head scratching to put together that it worked like normal positional params to the yield helper.

I'm not sure how to best improve this (maybe different variable/param names since title/header/body get repetitive and possibly confusing?).

@rtablada

This comment has been minimized.

Copy link

rtablada commented Mar 8, 2019

Playing around with the HBS a little I came up with this possible example (slightly modified from the existing example:

<Article @article={{this.article}}>
  <:header as |title author|>
    <h1>{{title}} by {{author.firstName}}</h1>
  </:header>
  <:body as |post|>
    <div>{{post.body}}</div>
  </:body>
</Article>

<article>
  <header>{{yield @article.title @article.author to='header'}}</header>
  <section>{{yield @article.post to='body'}}</section>
</article>

This not only decouples some of the more repetitive naming (especially @article.body to="body") but it also shows yielding multiple values.

@jessica-jordan jessica-jordan referenced this pull request Mar 11, 2019

Merged

The Ember Times No. 89 - March 15th 2019 #3

9 of 15 tasks complete
@jgwhite

This comment has been minimized.

Copy link

jgwhite commented Mar 16, 2019

Does this RFC propose that block names MUST be unique? i.e. would the following cause a compile-time error?

<Article>
  <:byline>Alice</:byline>
  <:byline>Bob</:byline>
</Article>
@Exelord

This comment has been minimized.

Copy link

Exelord commented Mar 17, 2019

I think we should allow on duplications. It may be useful to display the same content in different places with different format.

Eg you may want to display article's created At in header and footer. So you will be able to configure one block to see the result in both places.

@jgwhite

This comment has been minimized.

Copy link

jgwhite commented Mar 17, 2019

@Exelord interesting, could you provide a code snippet to go with that example?

@Exelord

This comment has been minimized.

Copy link

Exelord commented Mar 17, 2019

Oh sorry, you are actually right! But I think in that case it should just overwrite the prevoius one.

<Article>
  <:byline>Alice</:byline>

  {{#if isBob}}
    <:byline>Bob</:byline>
  {{/if}}
</Article>

The other important question (I think) that need to be clarified is the block's name format, as it is not clear from the example.

eg, it is allowed to name blocks like custom-block, custom_block, customBlock and then call them respectively <:custom-block>, <:custom_block>, <:customBlock>?

From angle bracket convention I assume the correct syntax, in that case, is {{yield to="customBlock"}} and <:customBlock>.
But what in case if someone will use {{yield to="custom-block"}}? Should the value be transformed or block will be not yielded properly?

@Exelord

This comment has been minimized.

Copy link

Exelord commented Mar 17, 2019

And I think we should also consider restricting else block calls to point always to inverse

<MyArticle>
  <:else>
    No article :(
  </else>
</MyArticle>

will equal

{{#my-article}}
{{else}}
  No article :(
{{/my-article}}

Otherwise, to keep the backward compatibility we would have to call <:inverse> block and have 2 different usage of the same component

@jgwhite

This comment has been minimized.

Copy link

jgwhite commented Mar 17, 2019

@Exelord that makes sense, though I think we’re talking about different (but related) things. My comment refers to the presence of multiple blocks with the same name, whereas yours refers to yielding to a single block multiple times. Both worth clarifying, for sure.

EDIT: In response to your edit in #460 (comment) — yes, that is an interesting case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.