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

Actions (aka Rails Service Objects) #71

Closed
wants to merge 2 commits into from

Conversation

tim-evans
Copy link

Rendered

This RFC originates from discussion on the ember-metal-injected-properties feature PR: emberjs/ember.js#5162 (comment)

@mixonic
Copy link
Member

mixonic commented Jun 29, 2015

Interesting. I have some other significant concerns, but the first in my mind is that these should likely not be called "actions", as that term already has a very specific meaning in Ember.

We would normally use an utils/ directory for things like this, and just import the function.

Perhaps there is a way for services with an actions hash to provide this functionality?

@tim-evans
Copy link
Author

Yeah, I called them actions because they acted like cousins to what Ember has called actions. This was good when it wasn't injected, but it does introduce complications with the naming scheme. And lots of confusion.

An action hash on services intrigues me. How would you propose developers to call the action from the service in that case?

I feel personally that what I call 'actions' here and Ember services have subtle differences. Services feel more appropriate for me in the cases of having to share a single instance across the application. A google analytics service is a good example of this.

Actions are injectable for the testing story. Otherwise, it would be simple to just import them from a file and call them with all their dependencies.

@davewasmer
Copy link
Contributor

Some prior / related art: https://github.com/davewasmer/ember-data-actions

@sandstrom
Copy link
Contributor

This RFC is also, at least partly, relevant: #32

@rlivsey
Copy link

rlivsey commented Jun 29, 2015

Big 👍 for this or something like it.

I currently have an app/use-cases folder where I export plain old JS functions to use in routes, eg:

import { doThing } from 'my-app/use-cases/things';

export default Ember.Route.extend({
  session: Ember.inject.service(),
  store: Ember.inject.service(),
  analytics: Ember.inject.service(),

  actions: {
    foo() {
      doThing(this.get("session"), this.get("store"), this.get("analytics"), ...);
    }
  }
});

This works fairly well and is easy to test, but without access to services you have to pass everything in, so injecting services would be really useful.

@mixonic
Copy link
Member

mixonic commented Jun 30, 2015

Services still provide the testability you are looking for pretty easily:

// app/services/do-thing.js
export default Ember.Service.extend({
  request() {
    return doSomeAjax();
  }
});
export default Ember.Component.extend({
  doThing: Ember.inject.service()
});
import Ember from 'ember';
import { moduleForComponent, test } from 'ember-quint';

moduleForComponent('do-thing', {
  integration: true
});

test('call a thing', function(assert) {
  var calledWith;
  this.set('doThingMock', Ember.Object.create({
    request() {
      calledWith = arguments;
    }
  }));
  this.render(hbs`{{do-thing doThing=doThingMock}}`);
  assert.ok(calledWith, 'request was called');
});

I would welcome a way to mock services in general. For example, this above does not mock a service accessed deeper in the render (a service on another component called in do-thing's layout section). An API for general service mocking in integration and acceptance tests would be very welcome, IMO.

Today, we often mock 3rd party APIs. It doesn't feel like a great solution despite our successes with it.

I think most of what is wanted here could be achieved with services today. The testing part of the RFC definitely interests me more than the closure-function-on-a-singleton part (in fact the lifecycle of "actions" is not addressed here, are they singletons of instance? Imagine an action calling itself again, is the state shared or are there two instances?).

@tim-evans
Copy link
Author

The testing story is actually what drove me to write this RFC. The pattern I've been using is very much like @rlivsey 's use-cases, and I found myself wanting to stub it out; it was a big pain to test the calling object otherwise.

As for the lifecycle, I consider actions to be stateless, which probably indicates that they are not singletons.

@tim-evans
Copy link
Author

@mixonic Any suggestions to get this moving forward?

(Also, naming suggestions. Actions are out, but what's a good name for this?)

@locks
Copy link
Contributor

locks commented Jul 9, 2015

@tim-evans aren't these just functions? what am I missing

@tim-evans
Copy link
Author

@locks The testing strategy is the main point of this RFC.

What I've done in the past is:

  • create a file that exports a single function in app/actions/
  • import that action in a file that uses it

But then testing becomes very difficult. How do I swap out dependencies in my app to test the behavior around them?

There's a few possibilities for this:

  • mock out the module loader to load a different file
  • add the function on the object as an injection

I think the latter is better than the former solution to this at the moment, so that's why the RFC is about injected functions.

@tim-evans
Copy link
Author

I'm going to close this RFC due to inactivity and having no path forward on this. Service mocking is available via emberjs/ember-test-helpers#96. I may come up with a solution internally that piggy-backs with the service architecture and follows a pattern like the helper shorthand API. If that amounts to something I feel is nice, I'll post an alternate RFC to that and link to this.

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

Successfully merging this pull request may close these issues.

6 participants