js module used as service token can lead to exception due to module cycles

[void-mAlex]

Following what the readme says:

The service token (a.k.a. ServiceFactory)

The "service token" is the second part of the logical key tuple for a service lookup. It is combines the string-based "name" of a service and the service factory in the traditional DI system. It serves a few purpose:

It uniquely identifies the service (implied by being part of the lookup key)
It tells the DI system how to instantiate the service
It links up the inter-module code dependencies

Typically, this token would be the service class, which makes sense as it fits all three of those purposes – it's clearly identifies the service, it's obvious how to instantiate the service from the class, and since you'll need to import the service class into the consuming module, it links up the code dependency as well.

there are various reasons why services will depend on each other either deliberately or accidentally.
this leads to a js exception
ReferenceError: While generating link to route "content": can't access lexical declaration 'SharedApiService' before initialization

this issue came up from trying to model services from an existing application that has a setup like

auth -> shared_api -> internal_api -> auth (cycle)

[chancancode]

Yeah that is unfortunate, there are many practical ways to resolve this, but they are not super elegant and I am not sure which if any we would want to recommend as the default, and I not sure that we want to preemptively encourage something more elaborate that avoids the problem.

Essentially you just need to split apart the token and the definition/implementation (which has the reference that requires the other services).

One way to do it:

export default abstract class MyService {

}

// I think this is good enough to break the cycle? If not wrapping it in a closure would work 
provide(Token, class ActualImplementation { ... });

Alternatively one/some of the services can forgo the declarators (which consumes the token values eagerly) and instead do it in the constructor this.otherService = lookup(...); which gets evaluated lazily and breaking the cycle

Don't love that you have to do either of these but they should work. Would like for there to be something more elegant

[pzuraq]

You could also do something like:

export default abstract class MyService {
  @service(() => Token) token: Token;
}

And have the closure run lazily on first access

[chancancode]

Would love to get some feedback from those who adopted it widely to see how often this is an issue, whether we should try to adopt a syntax that always avoid the issue (e.g. something like @pzuraq suggested), or make that optional and something you do if you encounter the problem, etc

[chancancode]

Another thing to say about this is I think the non-decorator form does not suffer from the same issue:

import Component from '@glimmer/component';
import { service } from 'ember-polaris-service';
import MyService from 'my-app/services/my-service';

export default class MyComponent extends Component {
  myService = service(this, MyService);
}

Overall, I personally prefer the decorator style, but if it is by a slim margin. Based on the current limitations in TypeScript and the current state of the decorator proposal, when using TypeScript the actual syntax required would be:

import Component from '@glimmer/component';
import { service } from 'ember-polaris-service';
import MyService from 'my-app/services/my-service';

export default class MyComponent extends Component {
  @service(() => MyService) accessor myService!: MyService;
}

(Arguably accessor is optional, and strictly speaking, without it would be more of an apples-to-apples comparison with the other snippet. Though, if you are going to type all the other stuff you may as well add the accessor keyword to make things lazy.)

Whereas – in the first snippet, that actually is the full TypeScript code because TypeScript can infer the type of the value.

So if we have to add more syntax to the decorator style, the function version may just edge out in this use case. (@pzuraq some of this maybe useful real world feedback for decorators, especially regarding their inability to provide type information in TypeScript.)

[pzuraq]

fwiw I don't believe that the inability to provide type info in TS is specifically because of the syntax or API, it should be possible to figure out what the type is in the future with the current proposal. Related issue: microsoft/TypeScript#52559

If I'm missing some context here I'd definitely be interested, I think being able to infer types from a decoration is very important.

[chancancode]

@pzuraq Yeah, certainly not an issue with the syntax itself, I think it's possible to design a TypeScript API for providing that information, but it just isn't there at the moment. I think this is an interesting case study that in theory would be a perfect fit for decorators, but after budgeting for the real world considerations, the non-decorator syntax end up being on par or arguably a bit more succinct.

The TypeScript limitation is certainly a big part of it (it's unfortunate that even after typing all of those extra characters, you ended up with less safe code than the function version). But with the laziness/cycle issue factored in (say we adopt the closure syntax), and the accessor keyword, it still kind of end up being a wash – the decorator version doesn't have the slightly awkward this in there, but you exchange that for the slightly awkward closure.

Not exactly sure what to do with that information/feedback, but maybe just an additional datapoint for you when other related issues come up.

[NullVoxPopuli]

anyone have a runnable example of this problem? I find the text / theory to be a little hard to follow, and some solid code in a stackblitz demo would be super helpful for me <3