-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
Ability to inject service into Template Only components #543
Comments
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 ModifiersThis 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 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 DefaultsThe 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 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: {{get (service "user-agent") "isNativeApp"}} Edit: @pzuraq beat me to the punch 😄 Edit: I published the |
@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];
}
} {{service "user-agent" "isNativeApp"}} Which was a bit less verbose, but had two significant down-sides:
It was a quite nice experience to fallback to the "do one thing well" philosophy, and let So far we've only been using the helper to read properties, but theoretically it could also be used to set properties, using your |
I published ⬆️ |
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 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, |
@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 |
Thought about this more. I think the helper approach is great, but I have two concerns:
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: {{let (lookup "service:foo-bar") as |FooBarService|}}
<button {{on "click" FooBarService.doSomething}}>Click</button>
{{/let}} This is already accomplishable of course, but then maybe the RFC is to make the On the question of symmetry, another q that comes up (and may already be answered) is that after #451, now that 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 |
To clarify, it definitely was not our intent to make the later example the recommended way going forward. In fact, the reason The reason decorators are better for defining injections is that they are inherently less dynamic. Using 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 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 |
That makes sense and seems reasonable to me. If Soooo, I guess that leaves us back at square 1, with the options of:
|
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! |
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.
The text was updated successfully, but these errors were encountered: