rfcs
rfcs copied to clipboard
set helper
Why would @model.someProperty
not be valid use case?
@btecu @model.someProperty
would be valid, but @model
alone would not. The issue is we don't know what property we're setting the value on. This is something mut
can do today that is pretty weird, and causes a lot of pain with in the VM.
@nightire
One thing always confuses me and my colleagues is that how ember.js determined the value passed to the one-argument version of {{set}} helper?
It does not. So when you do this:
<input {{on "change" (set this.foo)}}>
It will set this.foo
to the Event
.
This one would actually work. However I would strongly recommend you to not do this!
<Input @value={{@value}} {{on "change" (set this.foo @value)}}>
It only works because change
fires after input
. So this will be really problematic:
<Input @value={{this.value}} {{on "input" (set this.foo @value)}}>
Then this.foo
will always be one version behind this.value
.
But this is a limitation of the <Input>
component, not the set
(or mut
) helper.
I agree that this should be addressed. But this is out of scope here. A nice showcase
how this could be adressed is ember-box
. Or we could add an @update
parameter to
<Input>
that would allow <Input @value={{this.value}} @update={{set this.value}} />
.
But this is really out of scope here. Actually one of the reasons mut
is problematic
is that it does a lot of magic and tries to fix to many problems.
I'm very much in favor of set over mut, but a couple concerns:
-
mut
andset
both seem like a "shortcut" to just defining an action in JS and passing that around in the template. Is that correct or are there other reasons for using these? - If (1) is correct, then the main usage of these helpers would be for Template Only components.
Given (1) and (2) , requiring a context
in order to use this new helper begs the question: why not remove the helpers entirely and just require the action? Is it really so necessary to provide a primitive that generates functions in the template layer?
For what it's worth, I've never used mut
in all the time I've been using Ember, mostly because I never understood how to use it, and I don't feel like I've not been able to do something.
@mehulkar I would say yes to 1) but no to 2).
A good real-world example is using set
with power-select
:
<PowerSelect @selected={{this.destination}} @options={{this.cities}} @onChange={{set this.destination}} as |name|>
{{name}}
</PowerSelect>
Also notice you can use set
inside an {{#each}}
loop:
{{#each this.coreTeam as |teamMembers|}}
<SelectTimeZone @value={{teamMember.timeZone}} @update={{set teamMember.timeZone}} />
{{/#each}}
Now this is interesting because the alternative to set
is something like
@update={{this.setTimeZone teamMember}}
with that action:
@action setTimeZone(member, timeZone) {
member.timeZone = timeZone;
}
This is IMHO worse then set
, because the template has about the same size, but we dont know what setTimeZone
does before checking it, making it quite verbose. By using set
we clearly indicate this will just set a property and do nothing else. However in opposite to a bound variable with <Input @value={{this.data}} />
it is easy to refactor if we want to do more then just to set a property.
And I think the fact alone that mut
is used by a lot of people in real world applications means that we need set
.
@luxferresum the PowerSelect example makes sense but I'm not sure it's a good reason not to use an action in the JS instead. There must already be JS in order to use this.destination
. I think a better example would be:
{{#let (lookup (hash type="service" name="foo") as |someService|}}
{{set someService (get someService "destination"))}}
{{/let}}
In other words, something that didn't previously have a Component JS class, and would be able to keep it that way. To me, that seems like an edge case (and if that example above is any indication, a bit convoluted also).
The other example with each
also makes sense, but I'm not convinced that less verbosity is a good reason to introduce this into core.
That said, I still support this is as a pretty common sense replacement for mut
, for all the reasons you've pointed out. I would encourage deprecating mut
as part of this RFC.
FWIW we stopped using mut
some time back and had a general rule of "templates should not contain direct mutations". We also avoid addons such as ember-ref-modifier
for the same reason. Everything worked fine, though yes you did end up with some more one line actions to just set a value. I found the explicitness of this approach to be useful (less template magic), and because we use TypeScript we then also got some type checking on the assignment operation which was another benefit.
My 2 cents:
- I very rarely used
mut
, so don't really have a strong need forset
- I would prefer the magic-less versions, i.e.
{{set context "property"}}
(note the similarity to{{ref context "property"}}
), and drop the "2 argument" version in favor of the slightly more verbose use with{{fn}}
, as{{on}}
also does not allow partial application of arguments, but requires{{fn}}
for this - I think this could (and so should) be implemented as an addon, either a simple user-land addon, or - if the community agrees this should be part of the default Ember DX - as an "official" one part of the default blueprint, just as e.g.
ember-render-modifiers
There's some healthy discussion around this so I'd like to see if we can get a more definitive conclusion here. I'll bring this up with the rest of the core team.
The magic-less version is implemented in this add-on: https://github.com/pzuraq/ember-set-helper. I too am more inclined towards a magic-less version.
Here's a PR for a V2 addon version of the set helper with added types in case anyone here is using the set-helper would like to do a review of over in addon-land:
ember-helper version (<4.5 support, I guess): https://github.com/pzuraq/ember-set-helper/pull/50 plain-function version (>=4.5 support): https://github.com/pzuraq/ember-set-helper/pull/51
Thanks!
With the advent of gjs/gts imports, I have found the import for the set helper conflicting with the import of set from @ember/object.
I have to do the following all the time
import { set } from '@ember/object';
import setHelper from 'ember-set-helper/helpers/set';
Depending on where you export this from, feels like a inject as service
issue all over again. Dont have a suggestion, but I think set
as a name will yield conflicts.
@cah-brian-gantzler True. There are others as well, especially when plain-functions-as-helpers makes it more tempting to use wider-javascript-community functions directly in templates (e.g. there are a lot of function name overlaps between lodash and 'good old' ember functions).
That said, I have not had any use for import { set } from '@ember/object'
when working in octane-era ember. In what use cases do you use it? (as opposed to direct assignments of @tracked values?)
when converting older code I do not always rewrite all sets (even if var becomes tracked). But set is sometimes more convenient.
let propertyName = 'fieldName';
set(this, `child.${fieldName}.someProp`, newValue);
than
let propertyName = 'fieldName';
this.child?.[fieldName]?.someProp = newValue;
Really depends if you have variables that hold the property names.
Set is also used a lot in tests (and I like to do the tests as gts as well) because it effectively does a notifyPropertyChange, so you dont have to create full object with @tracked vars. Although the context change from this
to scope in the rendered template changes some things.
All in all though you are correct, its use is waning. My point was its far from gone.