-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
Named Blocks #226
Named Blocks #226
Conversation
What are your thoughts on the difference between this proposed syntax and the "real" WebComponents v1 concept of "named slots"? The idea there is that your component includes something like: <!-- my-component -->
<div class="a-wrapper">
<slot name="a">
</div>
<div class="b-wrapper">
<slot name="b">
</div>
<div class="default-wrapper">
<slot>
</div> and would be used like: <my-component>
<span slot="a">I end up in a-wrapper</span>
<span slot="a">I also end up in a-wrapper</span>
<span slot="b">I end up in b-wrapper</span>
<span>I end up in default-wrapper</span>
</my-component> I'm not sure how important it is to try to approximate the WebComponents version of the same idea, but it might be worth thinking about. For more information, the MDN docs on |
@alexlafroscia we might very well support the web components version in the future, but as it stands, it's not very composable; for instance, you wouldn't be able to forward a slot to another component for rendering (whereas in this RFC you can just pass along the block to whomever since the blocks are just Also, a lot of people (myself included) aren't fans of the behavior that everything that isn't selected out of your "block" because the "default" block. |
I presume that the lack of any Javascript-side API for the block values is intentional because we don't have any designed public API for them. It may be good to say so explicitly, and we should make sure the implementation marks any methods on them as private. Is The RFC focuses on the multi-block use case, but the first thing I would use this for is actually block capture of the default block, like for {{#to-elsewhere named="my-modal"}}
Hello world
{{/to-elsewhere}} by grabbing This proposal sneaks |
How is this related to glimmer's |
I'll leave my feelings of the currently proposed API in handlebars, as I think that the usage of Traditionally we have been using I find the
Could we use a different keyword? I'm thinking on the This: {{#x-foo as |a|}}
@default block {{a}}
{{as @header |h|}}
@header block. Referenced via @header in x-foo's layout
{{as @footer}}
@footer block. No block params.
{{/x-foo}} would become {{#x-foo as |a|}}
@default block {{a}}
{{#block @header as |h|}}
@header block. Referenced via @header in x-foo's layout
{{#block @footer}}
@footer block. No block params.
{{/x-foo}} The semantincs of the proposal seem reasonable, is the syntax what seemed unaligned with what we have today. |
We've taken in a lot of feedback over the last few days and it's clear that the <x-modal>
<@header>
<span class="special-header">
Header Text
</span>
</@header>
<@main as |c|>
<p>Modal Content {{foo}}</p>
<button onclick={{c.close}}>
Close modal
</button>
</@main>
</x-modal> This syntax has become a front runner because:
There's still quite a bit to resolve but we're feeling pretty confident about this syntax. If you're interested in mulling over alternative syntaxes, make sure you take a look at the graveyard of alternative syntaxes that have been considered as part of this RFC. |
@machty I'm guessing the following syntax would be invalid? (block params on the component's tag when in multi-block mode)
|
@csantero That's right. |
@machty do you envision people being able to do something like:
or maybe?
|
<x-modal @header=(component ...) /> Yes, the idea is that you can interchange between a closure component, a named block, and even a plain string. So you might start with: <x-modal @header="Some header text" /> Then realize you need more HTML for the header and update to: <x-modal>
<@header>
<div class="hero">
<img src={{heroImageSrc}}>
Some header text
</div>
</@header>
</x-modal> Then you might decide that you need an actual component to back the header and refactor to: <x-modal @header={{component 'my-special-header' headerImage=heroImgSrc}} /> In all of these examples, the layout of {{@header}} This same interchangeability does not exist (within this RFC) if you need to yield block params or hash arguments into the renderable. Currently, passing block params and positional params is only allowed for either named blocks or components, and passing hash arguments is only allowed with closure components. For example, invoking with positional params like this:
The above would issue a runtime error if This is an open question called out in the current RFC text, and could be even be addressed in future RFC's if we decide on the more conservative approach here... |
The latest proposal seems to imply that we should declarative way of defining blocks. Today we contextually and implicitly define them: <x-modal>
{{! Implicit block definition}}
</x-modal> In the proposal we effectively are defining macros that could expand into declarative calls to something like a <x-modal>
<@header>
<span class="special-header">
Header Text
</span>
</@header>
<@main as |c|>
<p>Modal Content {{foo}}</p>
<button onclick={{c.close}}>
Close modal
</button>
</@main>
</x-modal> Could be expanded/de-sugared into the following. {{#def-block header}}
<span class="special-header">
Header Text
</span>
{{/def-block}}
{{#def-block main as |c|}}
<p>Modal Content {{foo}}</p>
<button onclick={{c.close}}>
Close modal
</button>
{{/def-block}}
<x-modal @header=header @main=main /> This is out of the scope for this RFC but may paint how the implementation may play out and potentially how blocks could be reused in the future. |
My only concern with |
@cibernox yes, if that sort of syntax is what we want for dynamic tagname it would be |
I'm slightly confused because Ember currently does not have Does this mean this RFC depends on the new syntax landing in Ember first? Or does this RFC only apply to GlimmerJS until the syntax lands? |
@joukevandermaas shipping this RFC means fleshing out a basic idea of |
Do I understand it correctly, that this will basically will work exactly like this, and just uses a different syntax to specify block-arguments? And will we also get a way to use this with a helper? |
@luxferresum it will hit 99% of the use cases of that other RFC, but is a much more familiar/iterative/achievable API enhancement. What does "use this with a helper" mean? |
What's the state of this RFC? |
@knownasilya still under active development. Will follow up after this coming weekend's core team F2F, but here are a few mostly-agreed-upon developments that haven't yet been reflected in the RFC:
These seem to be the best compromises for backwards compatibility but by no means final and hopefully we can flesh it out a lot more over the F2F; will be updating the official RFC shortly. |
I just posted a pretty major update to the RFC, mostly fleshing out the details as to how Ember/Glimmer semantics diverge in some cases, and how component invocation syntax works with both component types. I feel pretty good about where it's at, but I really haven't gotten a chance to share the full details with the rest of the core team so keep in mind that some of the details are far from settled. (But please provide whatever feedback comes to mind!) |
text/0000-named-blocks.md
Outdated
|
||
Hence, the blocks provided in classic single-block syntax should also | ||
be exposed as properties (Ember) and args (Glimmer), and should have | ||
conventional, meaningful names names: instead of "default" (which is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
names names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
names named names
text/0000-named-blocks.md
Outdated
combinatoric mess of block syntax + `if hasBlock` checks to forward | ||
blocks through to the inner component) | ||
|
||
I propose an opt-in API; any Ember Component that wants `main`/`else` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean components with default and inverse blocks like today?
Do I understand correctly that you propose to extend all of them with a snippet below? Sorry if I misunderstood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Basically, you could create a component like this, and just extend any of your components from that one.
text/0000-named-blocks.md
Outdated
|
||
How should we handle `{{@header some blockparam data}}` | ||
when `@header` is a non-"callable", simple value like a string? Should | ||
we loudly error? Should it just ignore those "extra" args and render |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about having a debug flag, like the ones in config/environment.js
, and by default fail silently. But like the transitions debugging flags, output if the flag is set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addons will be using this syntax with its intended semntics and I don't think there'd be a way to prevent a config flag from falsely enabling logs on code that doesn't belong to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be an error IMO, similar to when you run {{component <something>}}
and <something>
cannot be resolved to a known component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this can be compared to the component case, which fails for the same reason injecting an unknown service should fail. This is IMO more like how {{yield}}
no-ops when hasBlock
is false; we don't require you to wrap all your yields in {{#if hasBlock}}
because, well, that would be miserable.
text/0000-named-blocks.md
Outdated
The `reopenClass` syntax s clunky, and I wouldn't be surprised if people | ||
wanted to use unified renderable syntax all over the place. So is there | ||
a less painful way to opt-in? Could it just be a property set on the | ||
prototype rather than the class object? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on the clunkiness of reopenClass
|
||
This might be desirable in the future, particularly for use cases | ||
involving flex-ish layouts where the component changes behavior / | ||
appearance based on whether blocks on passed in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty important for anything ambitious...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future we'll likely support local block declarations (so that you can pass the same block in to multiple components / multiple args of the same component; with something like this in place you could do something like {{x-layout footer=(if showFooter localFooterBlock)}}
. It's not great, but it's workable til we feel out all these ambitious use cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about an {{is-block
helper that returns true if the first argument is a block, false otherwise? Example:
{{#if (is-block @localFooterBlock)}}
{{@localFooterBlock}}
{{/if}}
If the consumer did not pass in localFooterBlock
, or if localFooterBlock
is e.g. a string, this would not render anything.
I have already built a similar helper for the biggest app I work on, where in certain cases we allow people to optionally pass components into other components:
{{#if (is-component localFooterBlock)}}
{{component localFooterBlock}}
{{/if}}
This generally works pretty well as a pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that'd work, but the idea behind unified renderable syntax is that all you should really have to do is check if the arg/property is truthy or not; it's either present or it's not, and you shouldn't have to check for the presence of a specific type of renderable (be it a block or component factory), as that would arbitrarily force anyone invoking your component to use only specific kinds of renderables.
Then again... the only way to build components that support universal renderables is to commit to always passing args to universal renderables as both positional params and named k=v pairs, which, is kinda weird/bad?
Elsewhere in the RFC I demonstrated this as {{header headerTitle title=headerTitle}}
, but maybe that's ridiculous to expect people to do, or maybe it only makes sense for addon authors to do, and app maintainers can just choose/prefer invocation styles that are consistent with the rest of the app? Will have to think on this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thinking on this more, I think what we can expect is that if we include named block param syntax as part of this RFC (and we probably should), then there will be a natural/organic shift towards using named KV args when universality is desirable. In other cases like control flow (e.g. a fancy-each
) component, yielding a positional param or two will remain a nice block-centric API for those kinds of use cases. And maybe in some cases there will be a mixture of positional and named params but I think generally speaking the API nudges folk toward named KV pairs, which is good.
text/0000-named-blocks.md
Outdated
@@ -307,6 +308,61 @@ case, nor would I want to have to introduce all of the `is-block` or | |||
guard. | |||
``` | |||
|
|||
#### Named block params |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
We're likely to FCP this on Monday. Per core team feedback I've refined a few things to clarify the Ember-centric nature of this RFC; spelling out a basic Glimmer roadmap is important but some details are likely to change over time, and hence Glimmer suggestions in this RFC should be considered non-normative. |
@machty To clarify, is the reason you're not going with: {{#x-modal}}
{{@header}}
My header
{{/@header}}
{{/x-modal}} For the curly syntax, because the |
@oligriffiths There's a few reasons we went with angle brackets +
|
Correct me if I'm wrong, but Given that |
@knownasilya @machty Sorry, I should have provided an example: <x-modal>
<@ember-power/select>
Header {{title}}
</@ember-power/select>
</x-modal> How do you disambiguate between whether |
Maybe named blocks can't have a |
We discussed this RFC in today's core team call, and decided it makes sense to merge it, with the caveat that since there are still open questions regarding the full story for Glimmer component invocation, there may be some slight changes (mostly likely syntactical, if any) along to way to shipping this feature, but that it didn't make sense to indefinitely block implementation of named blocks in Ember on waiting for 100% of the Glimmer story to fall into place. @tomdale's example above highlights a possible case where syntax might need to shift, but such cases might also be addressed by other means (e.g. perhaps an import mechanism or something along those lines). |
👏 🎉 @machty - Great work shepherding this through the RFC process. Thank you for working so hard on it! |
👏 👏 |
Is there a ticket tracking implementation of this feature? |
@chriskrycho No, named blocks needs some more design work to deal with angle bracket invocation. There's also some vague concerns about the idea and approach in general. The current proposal for named blocks doesn't have an answer for cases where you want to have multiple blocks with the same name or for situations where being able to nest blocks would be useful. |
@mmun, can you elaborate a bit on the concerns, or direct us towards those discussions? Once the RFC was accepted, we decided to hold off on some refactoring until it lands—and if there is a chance that it may never be implemented we will have to explore other options. |
@oliverlangan There is https://github.com/ciena-blueplanet/ember-block-slots which may be a viable alternative, while you wait for named blocks. |
@sandstrom |
@chilicoder we're using |
For anyone stumbling on this, implementation of yieldable named blocks is tracked here: |
Rendered
I presented some of the ideas behind this RFC at the EmberNYC meetup; here's a video of that. (The proposed syntax has since changed though)