Ability to inject service into Template Only components
mehulkar opened this issue Β· 15 comments
Templates that want to use functionality from services are currently forced to create a backing JS class for the injection. The downside of this is that once the class exists, it can be easy to stuff functionality into it. To keep simple components simple, it would be helpful to be able to do injections some other way.
It's possible that the template import primitives discussion in #454 can be part of this discussion.
I definitely agree that TO-components will pretty frequently want services in order to be a bit more flexible, and have been thinking of the best way to do this. There are 2 main possibilities that I think would be pretty straightforward.
Service Injections in Helpers and Modifiers
This is technically already possible today, with class based helpers, and with service injections in ember-functional-modifiers. This is also one of the more likely places that users will want to introduce services, so I think it would work out. You could imagine with a possible template-import syntax being able to do something like this:
import { helper } from '@ember/helper';
const isDarkMode = helper((theme) => {
return theme.isDarkMode;
}, { services: ['theme'] })
const toggleDarkMode = helper((theme, [event]) => {
theme.isDarkMode = event.target.checked;
}, { services: ['theme'] })
export default <template>
<div class="{{if (isDarkMode) 'dark'}}">
Dark Mode:
<input type="checkbox" {{on "change" toggleDarkMode}} />
</div>
</template>
You could also imagine creating a generic service
helper:
import Helper from '@ember/helper';
class service extends Helper {
compute([serviceName]) {
return getOwner(this).lookup(`service:${serviceName}`);
}
}
export default <template>
<div class="{{if (get (service 'theme') 'isDarkMode') 'dark'}}">
Dark Mode:
<input
type="checkbox"
{{on "change" (set (service 'theme') 'isDarkMode (get _ 'target.checked'))}}
/>
</div>
</template>
A bit verbose, but not bad!
Services in Argument Defaults
The other option I think would be to allow injecting services when providing defaults to arguments. This would require us to figure out how to specify argument defaults (see #479 for more discussion on that).
I'm not so much a fan of this approach, because then it wouldn't be easy to tell if an argument was actually an argument, or was a service that was being injected. I'd be worried that folks would try to inject services on class based components this way as well.
Other Options?
I can't really think of a simple/straightforward way to do this otherwise, but maybe we can think outside the box a bit here. It could be a new syntax for referencing services (e.g. the way @arg
references the arg named arg
, we could have a way to directly reference services). I think the key thing is figuring out how to reference them without using @
syntax or this
, since the first would be confusing, and the second isn't available in TO components.
Thanks for opening this discussion topic btw! I think this is a very important thing to figure out π
We use this helper:
import { getOwner } from '@ember/application';
import Helper from '@ember/component/helper';
import { assert } from '@ember/debug';
export default class ServiceHelper extends Helper {
compute([serviceName]: [string]) {
const service = getOwner(this).lookup(`service:${serviceName}`);
assert(`The service '${serviceName}' does not exist`, service);
return service;
}
}
It allows you to do stuff like:
Edit: @pzuraq beat me to the punch π
Edit: I published the {{service}}
helper as an addon: ember-service-helper
@buschtoens It's really cool to see that y'all are using it in real world apps though! Always great to design with actual feedback from usage, definitely curious to hear how it works out for your use cases, if you care to talk about it.
@pzuraq Sure! I'll ping you in Discord, when I have some time. π
Previously we were using it like this:
export default class ServiceHelper extends Helper {
compute([serviceName, propertyName]: [string, string]) {
const service = getOwner(this).lookup(`service:${serviceName}`);
assert(`The service '${serviceName}' does not exist`, service);
assert(
`The property '${propertyName}' does not exist on the ${serviceName} service`,
propertyName in service
);
return service[propertyName];
}
}
Which was a bit less verbose, but had two significant down-sides:
- no nested property access, e.g.
{{get (service "user-agent") "browser.version")}}
- changes are not observed π±
It was a quite nice experience to fallback to the "do one thing well" philosophy, and let {{get}}
handle the property access. π
So far we've only been using the helper to read properties, but theoretically it could also be used to set properties, using your ember-set-helper
, or to call methods. When calling methods, it's important to decorate them with @action
or alternatively use the ember-bind-helper
.
I published β¬οΈ {{service}}
helper as an addon: ember-service-helper
Using a helper is a great idea! My concern was that it adds another level of indirection, but I didnβt think to name my helper service
so that it would be more obvious.
That said, I think my original idea was something more aligned with app.inject. I generally think this API needs to be given a little more of the lime light, but not sure what the right answer here is.
IMHO, app.inject
should be deprecated and removed (there is another open issue here for that specifically).
@rwjblue are you talking about #508? Didn't see anything else searching with "inject" that was relevant. Would be interested to read that proposal/discussion because on the surface, I think app.inject
is a good primitive if DI is going to continue to be a thing. Based on the current programing model and seeing people come into Ember, I'd want to more explicit, if anything. (But I realize that this is a totally separate discussion and we should have it elsewhere!)
Thought about this more. I think the helper approach is great, but I have two concerns:
- it's asymmetrical to have to go through a helper, compared to injections in JS
- because it's not provided by core, it seems to do a disservice to Template Only Components, which I think are the way forward.
So it's possible that the RFC needed here is to add this helper to core, but that still leaves the first issue of asymmetry.
@mehulkar do you have a particular syntax that you would like to see? Even a rough gist would help here, itβs hard to talk about this in the abstract.
Something like this looks symmetrical-ish and makes sense to me:
This is already accomplishable of course, but then maybe the RFC is to make the lookup
helper a part of core?
On the question of symmetry, another q that comes up (and may already be answered) is that after #451, now that owner
is easily available, is there value in:
class Foo extends Service {
@service bar;
}
vs
class Foo extends Service {
constructor(owner) {
super(...arguments);
this.bar = owner.lookup('service:bar');
}
}
If the latter is the way forward, then a lookup
helper in core for templates makes even more sense.
To clarify, it definitely was not our intent to make the later example the recommended way going forward. In fact, the reason owner
is passed as the first parameter is to enable service decorations to work, since there isn't really a way to pass the owner out of band safely.
The reason decorators are better for defining injections is that they are inherently less dynamic. Using lookup
directly, you could do something like this:
class Foo extends GlimmerComponent {
constructor(owner, args) {
super(...arguments);
this.bar = args.useBar1 ? owner.lookup('service:bar1') : owner.lookup('service:bar2');
}
}
This means that to fully know what the service is intended to be, you will have to read the entire constructor
(or potentially the entire class definition, if users decide to lookup/assign services during different lifecycle hooks). Using a decorator, we can know that bar
will always a specific service.
This is much easier for users to read through and analyze, and for bundlers to read through and analyze (for a future world where we have Embroider, and the ability to bundle and lazy load assets more easily).
In general, the recommendation is to avoid using the owner
directly, and to simply pass it through so it's available on the class itself. I would personally avoid a lookup
helper for similar reasons. I would be more open to a service
helper, though I think it would have some of the same problems.
That makes sense and seems reasonable to me. If lookup
is not the happy path in JS, then I don't think recommending it in a template makes sense either. A service
helper is mostly syntactic sugar, so it just seems like a thinly veiled footgun than the footgun itself :)
Soooo, I guess that leaves us back at square 1, with the options of:
- special syntax for template injections (something comparable to decorator approach in JS)
- be ok with asymmetry and use a helper (either in core or user land)
It doesn't seem like there is a good path forward here that's different from the more generalized Template Imports in #454, so I'm closing this down. For the current use case a helper from userland works fine. Ergonomically, now that we have template colocation, I'm also ok with having a JS class for the sole purpose of service injection. I'm not sure a separate primitive in templates really solves any real problem. Thanks all for the discussion!