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

Merged
merged 1 commit into from
Apr 23, 2019
Merged

Conversation

wycats
Copy link
Member

@wycats wycats commented Mar 7, 2019

@rwjblue
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
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
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
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.

@pzuraq
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
Copy link
Contributor

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
Copy link
Contributor

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.

@jgwhite
Copy link
Contributor

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
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
Copy link
Contributor

jgwhite commented Mar 17, 2019

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

@Exelord
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
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
Copy link
Contributor

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.

@rwjblue
Copy link
Member

rwjblue commented Apr 12, 2019

We discussed this in todays Ember, and we are moving this into final comment period!

@rwjblue
Copy link
Member

rwjblue commented Apr 12, 2019

We discussed this in today's core team meeting, and are moving it into final comment period.

@rwjblue
Copy link
Member

rwjblue commented Apr 23, 2019

OK, y'all it's that time, let's do this!

@Kerrick
Copy link

Kerrick commented Jul 23, 2019

Does this have a tracking issue yet?

Copy link
Member

rwjblue commented Jul 23, 2019

Yep, over in emberjs/rfc-tracking#44

@btecu btecu mentioned this pull request Aug 30, 2019
@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Aug 20, 2021

It's been some time since this has landed in ember, and I'd say an absolute critical missing feature is the ability to forward blocks without needing to explore if/else has-block conditionals.

For example, say you are wrapping a component that has named blocks.
If the wrapped component has no markup then this is easy, you just:

{{!-- wrapper.hbs --}}
<Wrapped>
  <:block-a>
    {{yield to="block-a"}}
  </:block-a>
  <:block-b as |any number of args|>
    {{yield any number of args to="block-b"}}
  </:block-b>
</Wrapped>

But now let's pretend the act of passing a block at all adds additional markup (which is handy in many cases!), but for your wrapper component you need to ensure that you don't force the rendering of that additional markup when the consumer of your wrapper component does not pass those named blocks -- the wrapper.hbs now has to look like this:

{{!-- wrapper.hbs --}}
{{#if (has-block "block-a")}}
  <Wrapped>
    <:block-a>
      {{yield to="block-a"}}
    <:block-a>
  </Wrapped>
{{else if (has-block "block-b")}}
  <Wrapped>
    <:block-b as |any number of args|>
      {{yield any number of args to="block-b"}}
    </:block-b>
  </Wrapped>
{{else if (and (has-block 'block-a') (has-block 'block-b'))}}
  {{!-- NOTE: at the time of writing `and` is not a built in helper -- implementation is in progress tho --}}
  <Wrapped>
    <:block-a>
      {{yield to="block-a"}}
    </:block-a>
    <:block-b as |any number of args|>
      {{yield any number of args to="block-b"}}
    </:block-b>
  </Wrapped>
{{else}}
  <Wrapped />
{{/if  

This is especially deadly to productivity as the number of named blocks increases (which is very common in layout-style components).

The number of conditionals you need is the square of the total number of blocks.

So, if you have a max of 9 named blocks, you need 81 if/else conditionals to properly wrap that component with only 9 named blocks.
Now imagine if your component also takes a number of arguments... Your template files becomes huge.

I think in one of the named-blocks RFCs, someone suggested passing the named blocks kinda like arguments, but with the prefixing :. I think that'd be a great solution to have 1k-line boilerplate templates.

the above example would be:

<Wrapped :block-a=:block-a :block-b=:block-b />

or something.
one line <3

@NullVoxPopuli
Copy link
Sponsor Contributor

maybe a <Wrapped ...:blocks /> would be useful for this as well :)

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Nov 17, 2021

Here is an additional (maybe more concrete) demonstration and use case that came up with the need for this feature:
Using named blocks with component / dynamic content in i18n

(was working on this with @RuslanZavacky )
tl;dr: we want to be able to use components in translations.
So, today, we need to have this extraneous wrapper component:

<T @key="test-key" @label="my label" as |wrapper|>
  <wrapper>
     <:text>
       some text that is dynamic {{wooo}}
     </:text>

     <:link>
       <a href="#">link</a>
     </:link>
  </wrapper>
</T>

But it'd be ideal if we didn't need it at all:

<T @key="test-key" @label="my label">
  <:text>
    some text that is dynamic {{wooo}}
  </:text>

 <:link>
   <a href="#">link</a>
 </:link>
</T>

but we can't do that unless we can forward all blocks passed to T to the underlying component.
We can't do the technique mentioned in my previous comment because the named blocks are dynamic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Final Comment Period T-framework RFCs that impact the ember.js library T-templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants