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

Deprecate send API #609

Closed
mehulkar opened this issue Mar 30, 2020 · 13 comments
Closed

Deprecate send API #609

mehulkar opened this issue Mar 30, 2020 · 13 comments

Comments

@mehulkar
Copy link
Contributor

Are there any use cases of .send API anymore that can't be done any other way?

Since this is part of the action_handler mixin, mayybe it makes sense to deprecate the {{action}} modifier/helper instead, but I think using .send should be actively discouraged now?

@mehulkar
Copy link
Contributor Author

Crosslinking Eslint rule proposal as well: ember-cli/eslint-plugin-ember#753

@jherdman
Copy link

I have a few use cases left in my code, and all are simply because I haven't got around to a complete native class refactor yet.

@mehulkar
Copy link
Contributor Author

@jherdman what does native classes change regarding this? Also, is that path to getting rid of those usages clear? Would be good to know if there are usages that aren't possible (or hard) to implement without this API so an RFC on this can suggest alternatives.

@jherdman
Copy link

actions: {
  toggleFocus() {
    this.toggleProperty('isFocused');
  },

  myComplicatedAction() {
    // do some stuff
    this.send('toggleFocus');
    // do some more stuff

is the old pattern because you can't this.toggleFocus() for an action.

@Gaurav0
Copy link
Contributor

Gaurav0 commented Mar 31, 2020

I am in agreement that we need to deprecate the send api as part of deprecating non-closure actions. However, as long as non-closure actions are a thing, the send api is needed.

@jrjohnson
Copy link

@Gaurav0 it seems like deprecating send is a nice first step on the road to removing non-closure actions, not sure it needs to be part of a larger effort. I agree that it shouldn't be removed while those exist, but new code should be discouraged from using it and I think a deprecation does that well.

@Gaurav0
Copy link
Contributor

Gaurav0 commented Mar 31, 2020

People don't like deprecations, generally. We especially don't want to deprecate things where there is no clear path forward. Actually the way I'd attack this group of stuff is to deprecate action bubbling through routes first. Many people nowadays don't even realize that is there any more! Plus closure actions don't bubble.

@mehulkar
Copy link
Contributor Author

mehulkar commented Apr 1, 2020

Also an interesting alternative. I also learned the other day that action bubbling from controller to route goes by way of router:main, which blew my mind. I always assumed the controller was directly hooked up to its route. That behavior specifically seemed so nefarious that deprecating that first also makes sense.

Considering there are three approaches already discussed, I think a full approach probably will be best.

  • action bubbling
  • action helper
  • action modifier
  • actions “hash” (not sure how to handle this since it’s valid code. if action handling is deprecated, the only thing that can be deprecated is the binding of this, right?)
  • send API

@mehulkar
Copy link
Contributor Author

mehulkar commented Apr 1, 2020

@locks can we add the Deprecation candidate label here?

@btecu
Copy link

btecu commented Apr 1, 2020

Considering controllers are likely going away and classic components are eventually going to be opt-in only or something (where we prefer glimmer components), I'd rather not introduce unnecessary churn here. Once we have an alternative to controllers we can re-evaluate if send makes any sense for routes.

@mehulkar
Copy link
Contributor Author

mehulkar commented Apr 1, 2020

"unnecessary churn" is pretty subjective. Moving towards removing action handling is a good goal by itself for ergonomics and performance. I don't think it's necessary to think of it as part of controllers or classic components.

@rwjblue
Copy link
Member

rwjblue commented May 20, 2020

"unnecessary churn" is pretty subjective. Moving towards removing action handling is a good goal by itself for ergonomics and performance.

Agreed. Though I do think that an RFC that addresses this would do it as part of an RFC that deprecates "string based actions" and deprecating this.send would be one part of the transition path.

@mehulkar
Copy link
Contributor Author

That's fine with me. Closing in favor of #629.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants