-
-
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 Arguments Syntax ({{@foo}}
)
#276
Conversation
This bridges the gap nicely with the named block params described in #226. For example, with this RFC you could have a component with a template like The title: {{@title}}
The data: {{@data}} There is an analogous example with named block params: {{#x-modal as |@title @data|}}
The title: {{@title}}
The data: {{@data}}
{{/x-modal}} |
This is awesome. Good to see Glimmer.js ideas getting into Ember. Would the component invocation be the same as it is today?
Or would it be similar to what Glimmer.js has:
I feel the second one can help people see the relationship between of the arguments and it being used in the templates. |
wishful thinking that within the scope of this rfc:
i would really like to avoid a situation where people are writing code that isn't covered in the guides. imo, the guides should reflect what we should be doing / the best practices for a given ember version. i would also like to ease the path for addon authors who want to take advantage of this while continuing to support older versions of ember. creating an addon that converts newer syntax to older api would allow this in a relatively painless way (similar to the other polyfills). |
Yes, it would use today’s invocation system. |
Totally agreed.
Agreed.
I don’t quite agree here. We should definitely plan to deprecate using a provided arg without the
I agree generally speaking, but even if we did this the addon would have to be included by default (due to semver requirements). This isn’t a hard blocker, but it is a thing we need to be very careful about... |
text/0276-named-args.md
Outdated
As an aside, the ambiguity that causes confusion for human readers is also a | ||
problem for the compiler. While it is not the main goal of this proposal, | ||
resolving this ambiguity also helps the rendering system. Currently, the | ||
"runtime" tempalte compiler has to perform a helper lookup for every `{{name}}` |
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.
Spelling mistake: tempalte
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.
Témpalte, por favor.
Often developers like to put "default values" for properties (including "arguments" that may be passed in by the caller) on their component prototype. Componentclass CourseWidget extends Component {
isRequired: false // isRequired=false by default
} <h1>{{name}}</h1>
{{#if isRequired}}
<span class='required-indicator'>YOU MUST TAKE THIS COURSE</span>
{{/if}} Top-Level Template<!-- We can provide a value for the isRequired argument -->
{{course-widget name="Physics" isRequired=true}}
<!-- or not -->
{{course-widget name="French Literature"}} Is the intent to continue supporting and encouraging this pattern? Related: the React.Component#defaultProps approach to solving this is not terrible. |
Am I correct in assuming that positional params will be treated like named arguments in this proposal? I don't find any evidence of support for positional params in glimmer.js, so perhaps these won't be supported in Glimmer components in Ember? |
Fixed typos in RFC 276
@les2 I totally agree with you regarding documentation and only having one idiomatic way to do this. I think there's a bit of context missing from this RFC, though, that help shapes the right answer here. (To be upfront, we are trying a new strategy of focusing on small, very focused, very incremental RFCs. The downside is that the "big picture" plan is not as obvious, but hopefully the increased velocity is worth the tradeoff.) One thing we've been discussing is adopting an system like Rust's epochs. The tension we're trying to resolve is how you keep development fast, iterative and lightweight while still presenting a cohesive library to outside observers. With epochs, the idea would be to rapidly land a series of small, incremental changes related to improving some part of the system. In between those changes landing, the system may feel less coherent, or like there's multiple ways to do the same thing. Once a big enough set of related changes land that allow us to present a unified mental model, we declare a new "epoch" and use that time to create a snapshot of the programming model, update examples and guides, etc. There's way more details here and I'm probably not doing it justice, and I want to make sure I don't turn this into a meta-RFC on epochs and derail @chancancode's RFC. But I did want to chime in that one reason we may want to hold off on doing a guide pass immediately is there are a set of related changes we want to do incrementally, and we'll save the learning team a lot of work (and produce a more cohesive whole) if we wait for all of those to land before doing a big push. |
Because this RFC does not remove the auto-reflection feature of That said, there are several follow-on approaches we could use to help make the defaults use-case more ergonomic in the absence of auto-reflection. We would probably want to pick one of those approaches before actively encouraging people to avoid auto-reflection.
I think that you have identified an important issue for us to work on as we continue to iterate towards the full feature-set of Glimmer components. I also believe that it's important for us to continue to land the full feature-set, so we don't end up "stuck" in a particular in-progress state for very long. In the meantime, this RFC is a much more incremental improvement that improves the clarity of templates, and does not disallow the existing default patterns (as long as the template explicitly differentiates these use-cases by using |
How are arguments accessed in the JavaScript file of the component? |
This RFC provides a way to access the arguments in templates as an incremental step, but it does not remove (or replace) auto-reflection as the way to access the values from JavaScript. For now, I think I would expect a follow-on RFC to propose |
@sohara it is a plausible feature (something like Here are some reasons why it is not an obviously good/straightforward idea:
The only thing worth considering in this RFC is perhaps to reserve |
Agree. I think reserving |
|
I implemented the feature (behind a flag). You can try this on canary now: https://ember-twiddle.com/0c42585dc47fe38e118fffb9eec0714b?openFiles=templates.components.hello-world.hbs%2C |
I love the goal of separating attributes from a component's properties, but I'm concerned about the use of the
While the brevity of 1 symbol is appealing, it adds overhead to teaching and learning. Is there a technical reason to only use a symbol or is this for the sake of brevity? (My apologies if I missed that reason in the RFC). If it's only for the sake of brevity, my vote is to standardize on |
@scottmessinger a few things. First of all, I think "attributes" is pretty clearly not the correct concept (and it's the one we happen to use in Ember since 2.0 😱) because that concept has a related but not identical meaning in HTML. I think something like "arguments" is more appropriate. I also think that teaching components as "functions that take arguments" is an extremely high-level thing to teach someone. I have often taught extremely confused people about this mental model and seen them very quickly get a lot of clarity. I don't think a model that tries to hide the concept of "arguments" from components is very effective, because the concept of "components and functions" is so powerful. Similarly, I don't think that something like It causes people to believe that I don't prefer this explanation both because it's more complicated, and because it doesn't even provide a very good rationale for template-only components, which I am motivated to elevate in importance. TL;DR: I think adding a new concept of "arguments" is actually simplifying and clarifying, and helps to address some inconsistencies between reality and the ways people tend to think about the current API. |
2 cents: It would be nice if Secondly, as @mike-north also suggests, I've always used default / fallback properties (especially for defining noop functions for when closure actions aren't passed in, so that code doesn't fail with On that second note, I'm actually not clear on why it is important to differentiate between component defined properties and named args, since the pattern for "overriding" properties on objects already exists, even in |
The first problem with this approach is that the `{{name}}` syntax is highly | ||
ambigious, as it could be referring to a local variable (block param), a | ||
helper or a named argument from the caller (which actually works by accessing | ||
auto-reflected `{{this.name}}`) or a property on the component class (such as |
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 had no idea! I want to start using this yesterday.
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.name
? You can.
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.
Now that I think about it, I vaguely remember that {{this.name}}
was the right way to use properties in pre 1.0 or 1.x. Or something about {{this.name}}
would not be proxied to the underlying model of the controller and {{name}}
would be (or vice versa). But that is a side topic, don't want to take over this rfc 😄
My only comment is that if it makes sense, perhaps a few more words could be reserved? Something like |
One big reason is Glimmer Components will not have the auto-reflection semantics: you can get a glimpse of this now by trying this with #278 together on canary: https://ember-twiddle.com/0c42585dc47fe38e118fffb9eec0714b?openFiles=templates.components.hello-world.hbs%2C (Updated/fixed link) |
|
@locks i'm not surprised that it isn't possible right now (and that's great!), but my point is that as a beginner, I have no reason to believe that a string starting with |
This RFC was nominated and approved for Final Comment Period (FCP) yesterday, since most comments have already been replied to. The FCP with last for a week. If there are any substantial new arguments that are brought up during this time, the FCP will be restarted (or aborted). If you haven't reviewed the proposal yet, now would be a great time to do so. If I may, here is a summary of the main objections:
So, I will wait for @tomdale to comment and let him initiate the official Final Comment Period. Meanwhile, since the feature have landed in canary behind a flag, I encourage you to give it a spin and get a feel for it, either using this Ember Twiddle or by using a canary build with the |
Regarding sigils vs. reserved identifiers, I'm sympathetic to the arguments people have made against Our design philosophy is that you get a "sigil budget" that you must spend very carefully. As @wycats mentioned above, I think having the concept of component arguments with first class syntax (i.e. a sigil) to access them is a good thing; the syntax helps reinforce the concept. I'm also persuaded by his argument that not distinguishing arguments via Ultimately, I think we are safe to FCP this for the following reasons:
|
@wycats Thanks for the response!
I completely agree with this. "arguments" is more appropriate than "attributes" and "this.args.name" or "args.name" isn't a simplification as it's almost but not exactly right. If "arguments" is the powerful concept and the sigil
This suggestion builds on @mehulkar's idea.
FWIW, actually quite like the idea of using the
One advantage to using the word "arguments" is we can create something like Another advantage to using TL;DR: If "arguments" is a powerful concept and since the sigil |
@scottmessinger thanks for the thoughtful reply. I want to reiterate that I think the best way to understand Ember components is as functions that you're calling with arguments. So when you say: To put it directly before diving into some more details, I don't think it would help JavaScript if people understood Some details follow. Components Have No Signatures (Yet)Unlike functions in most languages, today's components have no "function signature". You could imagine that a component with this body: <h1>{{@title}}</h1> Actually looks like this behind the scenes: {{#def-component as |@title|}}
<h1>{{@title}}</h1>
{{/def}} where This is more or less how Glimmer works internally (the parser finds all of the Another way to put it is that when you use I personally don't like the fact that there's no explicit way to write this signature, and really want to be able to (for this and many other reasons). In the meantime, it's best to think of components as automatically taking any Glimmer Components Need Named ArgumentsThe reason Glimmer components mostly take named arguments rather than positional arguments (as JavaScript functions do) is that they are intended to be embeddable inside of markup that looks like HTML: <ArticleBody @title={{name}} /> This means that our "arguments" are almost always named arguments in practice. Because of the lack of an explicit signature in Glimmer components, we use a sigil to distinguish between named arguments and other names in scope (which include positional arguments from APIs like Let's Not Couple All of the Improvements TogetherWe've occasionally considered a large, sweeping overhaul of the system: add mandatory signatures to components, eliminate But increasingly, this made the entire package impossible to ship incrementally, both in terms of the total design and implementation effort, but more importantly in terms of getting the community to incrementally adopt and provide feedback on the new features. I personally believe that to regain some velocity on this package of features, it's important for us to find ways to ship parts of the feature set of Glimmer components incrementally but quickly. This may mean that in the interim, some of the landed features can only be understood in terms of a delta from the previous features, but I don't think we should allow that to stop us from landing features as we are able to implement them. That said, I do think it's critically important for us to have the rough strokes of the full feature-set up front, and for us to be prepared to land an atomic update to the mainline guides that reflects the programming model of all of the features in the near term. This is a near-term compromise that allows us to make reasonable progress on landing long-desired-but-delayed parts of the Glimmer programming model in shipping versions of Ember. I hope that made sense 😄 |
Thanks everyone for your participation! Since no major issues were found during the FCP, we will now close and merge this proposal. |
@chancancode The "Rendered" link in OP 404s. Can you please update it to the merged proposal? |
@Kerrick Thanks. Fixed it. |
Rendered