-
-
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
unique-id helper #659
unique-id helper #659
Conversation
I think calling this helper <button id={{id}}>my button</button> This perfectly valid piece of code will now break, because <button id={{this.id}}>my button</button> But I would argue this type of issue can be pretty hard to track down. Maybe something like |
I think a less generic/general name would be better. For example, I think the previously proposed |
Agreed for both backwards compatibility and readability reasons. I'd propose |
Oh, haha, just read #612 (comment), which I hadn't before. I guess that's corroboration that |
Yeah I agree this is a problem we need to solve, but I'm not sure picking a different name will actually solve the problem (though it may make it less likely to occur). Any other name we could come up with would have the same potential problem colliding with existing controller/component properties. Admittedly, other names we might choose (eg. The argument-less helper invocation proposed here may be problematic for this reason, and it will be complicated by template strict mode, which will require mandatory parens to invoke an argument-less helper. |
I think this is something we could find out- let's do a code search in Ember Observer and ask the community to weigh in on it (via Discord, Twitter, etc). That way we could choose something that won't be problematic for most use cases. What do you think? |
Looks like It also may be worth considering the names |
What's the benefit of terseness? I understand that some built-in helpers are pretty terse, but we have |
In any case I strongly agree with the comments that However I do like the idea to also have the AST transforming variant in ember itself. I currently have an addon that basically implements what is proposed as alternative here. And its IMHO much more ergonomic to not require additional nesting. |
This would save a lot of boilerplate for me. Without I do this quite frequently: export default class MyComponent extends Component {
guid = guidFor(this);
} {{#let (concat this.guid "-input") as |id|}}
<label for={{id}}>Name</label>
<input id={{id}} …>
{{/let}} 🤔 I guess that's not so bad. 🤷 |
I've just updated this RFC to use the helper name |
👋 We discussed this in today's core team meeting. Overall we are 👍 on the direction here, but would like to see a couple of (hopefully) small tweaks:
Thanks again for pushing on this @steveszc, hopefully the last round of tweaks 🤞... |
@rwjblue Thanks for taking the time to review with the core team!
If the default/only invocation option is the argument-less form, then I see two possible APIs that we'll need to choose between.
Did the core team meeting discussion assume that one of these two options would be the preferred API? It seems to me that this RFC will need to articulate which of those two APIs is being proposed. |
IIRC, we want to be explicit in the RFC that use of the helper will return a unique ID on every invocation, and that we were okay with @steveszc does that help? |
Maybe I've missed something, but if all this helper does is return a unique id, why does it need to be in Ember core? export default buildHelper((_args, _options) => {
return Math.round((Math.random() * 10 ** 20)).toString(36);
}); I thought the original idea was to make id generation for form id/for pairs easier. That would have been useful! (I'm still positive to that idea) Now it's just a vanilla JS function wrapped in an Ember helper, not too different from Spending core-team cycles/effort on a JS one-liner doesn't seem worthwhile to me. |
Yes, this is the form we want to go with. |
Good question! Ultimately, it does not need any privileged access or other things that are only available internally (and therefore "need" to be in internals). The reason it is critical that we ship this by default (in one form or another) is that it is imperative that it be available as part of the main Ember programming model. I personally don't care if that is via an addon that is included in the default stack (and it may well turn into that when we can actually use template imports; we could add a thing like |
@rwjblue Alright, I can see some benefits in shipping even the weaker version, but I would have preferred one that did more than simply generate a random string (the earlier proposal with uniqueness based on an object). But it's no biggie 😄 |
@rwjblue @MelSumner I believe the core team feedback has now been addressed. |
We have discussed this in today's Framework Core Team meeting, and decided to move this RFC to Final Comment Period. |
Just voicing support for this RFC and the proposed helper. It'll be incredibly useful in the apps I work on every day and I'm glad to see it coming to the core framework. Thanks to everyone involved in getting it to this stage. 🎉 |
One thing we have to make sure to do is that when we land this feature that we explicitly add tests for the "real world snippets" that this RFC mentions (to guarantee that they work as specified), but I guess that we all assumed that anyways.... |
We chatted about this again in todays core team meeting, and are 👍 on moving forward! |
Does an installable polyfill for this exist yet? |
@chriskrycho |
What is the implementation status of this RFC in the framework? |
unique-id landed in 4.4: https://github.com/emberjs/ember.js/releases/tag/v4.4.0 |
Thx a lot for the update @NullVoxPopuli I always find it hard to find where/when a RFC has made it to the framework… For example, in the actual rendered version of this RFC , I can see the following at the top Is this the right place to search for this info? |
@bartocc
(viva la chocolatine ✌️) |
Thx @MrChocolatine , I do follow the blog and I had missed the new feature in 4.4, but the releases subscription is a great tool 👍
😆 |
Advance RFC #659 "unique-id helper" to Stage Recommended
See pre-RFC issue #612
Rendered