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

Intent to RFC: Deprecate {{mut}} #538

Closed
pzuraq opened this issue Sep 5, 2019 · 13 comments
Closed

Intent to RFC: Deprecate {{mut}} #538

pzuraq opened this issue Sep 5, 2019 · 13 comments
Labels
Seeking Co-author T-deprecation T-framework RFCs that impact the ember.js library

Comments

@pzuraq
Copy link
Contributor

pzuraq commented Sep 5, 2019

  • Proposed Target: Ember v5.0.0
  • Alternative: Manually update actions, userland helpers

mut is a very confusing helper. If you pass it down directly, it gives you the value it references, but if you give it to the {{action}} helper, it turns into a setter that updates that value.

<button {{on "click" (mut this.clickEvent)}}>This will not work</button>

<button {{on "click" (action (mut this.clickEvent))}}>This will work</button>

In addition, the fact that you have to curry values after the helper is generally a sticking point in teaching users how to use it:

<button {{on "click" (action (mut this.greeting) "Hello")}}>
  Set Greeting
</button>

The mut helper has been considered not to be the best practice for some time now, and removing it from the framework will help to prevent confusion in the future.

Migration Path

The official recommendation for replacing mut will be to manually write actions which update values:

<button {{on "click" (fn this.setGreeting "Hello")}}>
  Set Greeting
</button>

This is a fair amount more boilerplate, but given the confusing nature of the mut helper, it is less likely to cause issues in the long run in existing applications.

Since this is not the most ergonomic solution, we do expect there to be experimentation with better patterns in the ecosystem - ember-set-helper is an example of one such possibility, and others are absolutely encouraged. In time, we can RFC a better helper or pattern into the core of Ember.

Deprecation Timeline

mut has not been considered best practice for some time, but its usage is not uncommon in the ecosystem, and users who do use it appear to use it heavily. There also are not many existing alternatives in the addon ecosystem, so it's better to give a longer timeline for the ecosystem to adapt. Ideally, we'll be able to introduce a direct alternative before mut is fully removed.

@webark
Copy link

webark commented Sep 5, 2019

agree with the 5.x timeline. This was the prescribed way for a while, and having migrated off some of the old one way controls recently to this, just to have to do another one due to the shifting prescribed behaviors would be tedious.

@Turbo87
Copy link
Member

Turbo87 commented Sep 5, 2019

I would prefer it if we offered a built-in ergonomic alternative before deprecating the existing solution.

A lot of codebases are using mut to set values in templates, and without a ergonomic alternative the migration will be quite painful. If mut was deprecated there should be a codemod to transform it to whatever the recommended solution is.

@balinterdi
Copy link

@Turbo87 ember-set-helper could be that ergonomic alternative.

@Turbo87
Copy link
Member

Turbo87 commented Sep 5, 2019

@balinterdi yes, but IMHO we should try out that addon, see if it can be that alternative, if it works pull it into core and then deprecate mut. Leaving core without such an ergonomic alternative seems bad to me.

@frank06
Copy link

frank06 commented Sep 5, 2019

set looks better but I never found mut problematic

Copy link
Member

rwjblue commented Sep 5, 2019

I would prefer it if we offered a built-in ergonomic alternative before deprecating the existing solution.

@Turbo87 - Ya, I think this is reasonable.

@pzuraq
Copy link
Contributor Author

pzuraq commented Sep 5, 2019

if it works pull it into core and then deprecate mut. Leaving core without such an ergonomic alternative seems bad to me.

I definitely understand the worry here, this is a very common pattern and it will be painful to migrate for heavy users. If we didn't have the ability to design and build solutions in userland at all, I would definitely agree.

The reason I'm a bit more aggressive here is, {{mut}} is a known antipattern. I've heard it actively discouraged by core for over 2 years now (at least since I started working on native class syntax). But, many users have still adopted and used it in all this time, and we didn't get any experimentation even though it was possible. I think in part this is because despite its issues, {{mut}} was seen as something in core, and thus it was safe.

I was actually just discussing this in Discord with @balinterdi, where he was planning on using {{mut}} in a new book, and we recommended instead that he show examples with full, manually written actions. I feel like at this point, it's really causing more harm than good for it to exist in core. We don't include it in any documentation or guides, other than API docs, and we are actively discouraging it.

Historically, we had other cases where this happened, like observers. But in those cases, userland solutions were not possible - here, it is. So, we could move a bit faster, and possibly prevent more damage from users adopting the antipattern.

Alternatively, maybe we could create a new type of warning system, like a pre-deprecation system that users could opt out of. The main thing I personally don't want to gate on is having a solution in core, because I believe this will both disincentivize experimentation, and cause more damage to the ecosystem. (Edit: I do think waiting for v4 and extending the deprecation to v5 would be fine though too)

@webark
Copy link

webark commented Sep 5, 2019

{{mut}} is still in the guides https://guides.emberjs.com/release/templates/input-helpers/

To achieve this you need to use the {{get}} and {{mut}} in conjunction

To the point of

{{mut}} is a known antipattern. I've heard it actively discouraged by core for over 2 years now (at least since I started working on native class syntax).

The first I've heard that it was a "known antipattern", and "actively discouraged by core" was just a few months ago during a conversation about using the built in "input helpers". For a definite set time (at least from my understanding) the preferred default way to deal with form controls was to use mut. I understand the change, and it's not that I don't welcome it, the "known antipattern", and "actively discouraged by core" was not something that was laid out or communicated as clearly as something like observers or even events.

@pzuraq
Copy link
Contributor Author

pzuraq commented Sep 5, 2019

@webark precisely, that's actually the main issue here IMO. Every time communicating this would come up in conversation, it would inevitably get push back because "we don't have an alternative yet", and then because we couldn't agree on alternatives. Add to this that there hasn't been any experimentation in addon space for us to test out, and it really becomes a difficult chicken-and-egg situation.

I just checked and realized it's actually still in the mainline guides, which is really disappointing 🙁 I definitely think we should remove it for Octane's guides at least. Given the conversation here I'm going to update the pre-RFC to suggest v5 as the timeline instead, definitely think it will take longer than we'd hoped to get off of this helper.

@balinterdi
Copy link

balinterdi commented Sep 5, 2019

To second @webark, I also only recently learned that mut is not the way of not having to define an action handler (see my blog post) – although at the time there wasn't anything better.

And I only learned this week that its use is actively discouraged. The currying behavior has always been funky and I've found it difficult to teach.

My main issue, however, is the inconsistent behavior that Chris describes in the very first sentence:

mut is a very confusing helper. If you pass it down directly, it gives you the value it references, but if you give it to the {{action}} helper, it turns into a setter that updates that value.

With some idioms landing in stable Ember (notably on and fn) that weird behavior surfaces way more easily now that you don't have to use action in your templates.

@balinterdi
Copy link

I was actually just discussing this in Discord with @balinterdi, where he was planning on using {{mut}} in a new book, and we recommended instead that he show examples with full, manually written actions. I feel like at this point, it's really causing more harm than good for it to exist in core. We don't include it in any documentation or guides, other than API docs, and we are actively discouraging it.

FWIW I converted the book's examples to use the set helper from ember-set-helper in most cases and it works great and is way more intuitive I think. It seems like a codemod shouldn't be very difficult to write.

@pzuraq
Copy link
Contributor Author

pzuraq commented Sep 6, 2019

Just published ember-simple-set-helper as another, much more direct alternative to mut, for those of you who may have been spooked by placeholder syntax in ember-set-helper.

I definitely encourage others to experiment here as well - I think the baseline is something that's basically the same as mut, which has advantages and disadvantages, but I also think we might be able to do better 😄

@pzuraq pzuraq added T-deprecation T-framework RFCs that impact the ember.js library Seeking Co-author labels Oct 8, 2019
@pzuraq pzuraq changed the title Pre-RFC: Deprecate {{mut}} Intent to RFC: Deprecate {{mut}} Oct 8, 2019
@pzuraq
Copy link
Contributor Author

pzuraq commented Oct 8, 2019

After much discussion and deliberation, I'm closing this issue since it turns out that there is not as much consensus around the future of mut as I originally thought. For more information, I wrote a detailed blog post that goes into the original design of mut, the reasoning behind it, and its current state.

Thanks to everyone for commenting and discussing, the community's feedback was invaluable here, it really got us talking on the core team and helped us to clear some things up!

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

No branches or pull requests

6 participants