chancancode / ember-polaris-service

Previewing Ember Polaris style services
Other
13 stars 3 forks source link

engines #5

Open chancancode opened 8 months ago

chancancode commented 8 months ago

document patterns for what's accomplished today with engines, and make sure this does what they need @void-mAlex

void-mAlex commented 8 months ago

would it help if I set up a monorepo with the use cases (read edge cases) that exist today?

chancancode commented 8 months ago

@void-mAlex definitely wouldn't hurt if you have time for that; ultimately we need to enumerate the use cases and see if they are solved satisfactory. a good starting point would be to read what's written here and see what else you think is missing

void-mAlex commented 8 months ago

@chancancode I have been trying to understand the new proposal beyond the section specifically about the engines/scoped services linked above I will start the conversation with saying that wrt engines the fact there is an owner with a destructor on it is part of the problem

if an engine is the first to DI the service will it be the one to have the owning scope?

that is one of the problems with todays engines creating duplicates of a service under the correct circumstances due to engine creating one in its scope before the app DI into its scope and has a chance to pass the service to the engine (actual bug that I worked around by injecting the problematic service into the application route so the app always gets to create it in its scope first - even if it's not needed there)

could the lifecycle hooks be tied to the symbol for the service instead?

please ask whatever followup questions since these thougths are pretty raw and I can't decide what info to offer that could definitely make things clear

chancancode commented 8 months ago

The presumptions are that engines do not need to have a separate owner anymore in the new design (and in general, engines don’t really have to do much anymore, with route based code splitting etc taken care of by and falling out of the natural patterns).

The reason engines needed to be a separate owner/child container to the application is specifically for service isolations (and, in the past, other DI objects too, but they don’t exist anymore in this world).

In the past would you make sure engine instantiated objects have their owners set to the engine, and that is specifically so that service:store in the app and within the engine are completely different things, and engines/apps don’t accidentally pollute each others’ namespaces. Since the second part of the key is a string, it would have been quite easy for them to collide by accidentally using the same string. So we solve that by changing the first part of the lookup key (the owner).

In this new design that is accomplished and solved by replacing the second part - the string key - with object values. Which means you can trivially make them distinct between the app and engine. In fact they will be distinct, since every object/class is different (!==), so you won’t be able to share anything unless you go out of your way to share these objects with some sort of coordination, like creating a common peer dependency that defines the well-known token for the service, or stashing it on a global variable.

So with the second part of the key being distinct by default, the thinking is there is no need to force the first part of the key (owner) to be different between apps and engines just to accomplish the isolation.

void-mAlex commented 8 months ago

if engines don't have their own scope then how will a service that is purely used/created by the engine be destroyed ? second if you take v2 app format to a place where it's just a federated module that runs in a host that provides 'vendor' code then engines are nothing more than an app, it's just easier for people transitioning from todays engines to call them as such.

for your statement that engines don't have scope then that would mean app don't have scope, and if that's true when does the service get cleaned up?

In this new design that is accomplished and solved by replacing the second part - the string key - with object values. Which means you can trivially make them distinct between the app and engine. In fact they will be distinct, since every object/class is different (!==), so you won’t be able to share anything unless you go out of your way to share these objects with some sort of coordination, like creating a common peer dependency that defines the well-known token for the service, or stashing it on a global variable.

I believe it's common to have a shared peer via an addon (which is exactly the situation I am finding the service bug mentioned in my previous comment)

app
  - api service 1 -> injects sharedapi service from addon
engine
  -api service 2 -> injects sharedapi service from addon
addon
  -sharedapi service
chancancode commented 8 months ago

if engines don't have their own scope then how will a service that is purely used/created by the engine be destroyed ?

for your statement that engines don't have scope then that would mean app don't have scope, and if that's true when does the service get cleaned up?

They will, like everything else, inherit/share the same scope (the app), and be destroyed when the app is destroyed.

Do engines not have the same (or <= to) lifetime as the app today? I didn't think engines can be "unmounted"/destroyed prior to the app being destroyed. Once created, I thought they live as long as the app. Is that not true?

I believe it's common to have a shared peer via an addon (which is exactly the situation I am finding the service bug mentioned in my previous comment)

In the case where the engine and the app intend to share the service, the approach of defining the "well-known" service token in a shared addon would work just fine. If the engine did not intend to share, then it can create a nested scope.

Either way, the lookup() function takes exactly two argument, the scope and the token, both of which can just be arbitrary object. It's basically just a fancy WeakMap that uses the tuple of (owner, serviceToken) to uniquely identify the service instance. As long as those two inputs are the same, you'll get the same instance.

void-mAlex commented 8 months ago

Do engines not have the same (or <= to) lifetime as the app today? I didn't think engines can be "unmounted"/destroyed prior to the app being destroyed. Once created, I thought they live as long as the app. Is that not true?

for routable engines they will unload when the route exists, I would need to see what is leftover today, but what I would expect is for all of it to be gone

void-mAlex commented 7 months ago

I'll use this repo for more experimentation as it sets up some more complex scenarios that one could encounter https://github.com/void-mAlex/ember-polaris-service-use-case-test

void-mAlex commented 7 months ago

the major issue that can be seen in the repo is that the previous way of passing services to the engine no longer works and the importing of the service goes against intent of the feature of engines that are supposed to receive instantiated services