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

Registry vs Resolver priority #43

Open
ef4 opened this issue Apr 14, 2017 · 6 comments
Open

Registry vs Resolver priority #43

ef4 opened this issue Apr 14, 2017 · 6 comments

Comments

@ef4
Copy link

ef4 commented Apr 14, 2017

The readme says

the container in this example has both a registry and a resolver. The container will look for classes in both, but the registry always takes precedence. If the registry is empty, the container will fall back to asking the resolver for its help.

But I am seeing the oppposite behavior here in the container and here in the tests.

Is this a bug in the code or in the docs?

@dgeb
Copy link
Member

dgeb commented Apr 14, 2017

Good catch. We should discuss. The code and tests match the behavior in Ember, but I find it easier to rationalize overrides that can be manually registered in the registry.

/cc @krisselden @tomdale

@ef4
Copy link
Author

ef4 commented Apr 14, 2017

Yes, I was hoping the README was the correct one, it makes more sense to me that way too.

@mixonic
Copy link
Member

mixonic commented Apr 14, 2017

Ember uses the registry for defaults which the user can override by creating a file on disk at the right location. We need something that satisfies that mechanism. You could define a new default resolver for example.

We should keep this use case in mind if we make a change. I would be sad to change it based on feels and break the compatibility path with Ember (we're pretty close, I have Ember working with glimmer/di in a branch).

Additionally it is worth noting the behavior has already slightly diverged from Ember. In @glimmer/di, the container manages both the resolver and the registry.

Some context: Registries can have fall-back registries. For example in Ember we use fallback registries so the registrations for application are done once while registrations into applicationInstance are unique to that instance.

Because in @glimmer/di the container manages the resolver, the check for a factory is:

  • Ask the container
    • Ask the container's resolver
    • Ask the container's registry
      • Check registry's registrations
      • Ask the registry's fallback registry
        • Check the fallback registry's registrations

However in Ember, the registry manages the resolver. For an Ember application the check is:

  • Ask the container
    • Ask the container's registry (application instance registry)
      • Ask the registry's resolver. In Ember, there isn't a resolver here.
      • Ask the registry's registrations
      • Ask the registry's fallback registry (fallback is the application registry)
        • Ask the fallback registry's resolver (In Ember, the resolver you configured i.e. DefaultResolver)
        • Ask the registry's registrations

So a direct port of @glimmer/di to Ember will change lookup slightly:

  • In a @glimmer/di port, resolutions will override registrations in any registry
  • In Ember currently, registrations in the application instance registry override resolutions but resolutions override registrations in the application registry.

@ef4
Copy link
Author

ef4 commented Apr 14, 2017

Thanks @mixonic. I would certainly not want to make integration harder. It sounds like we should just update the README to match the existing code.

@tomdale
Copy link
Contributor

tomdale commented Apr 19, 2017

If it's possible, I would really love to find a way to make the registry take precedence over the resolver. Generally speaking, in programming the explicit always overrides the implicit. The fact that it's switched here is a stumbling block for many people digging into the Ember DI system, myself included. (Clearly, since I wrote the README and got it wrong. 😄 )

That said, I don't want to make Ember integration onerous. Ember has to do some contortions internally to handle the fallback case, so I think switching this might actually clean up some app initialization code inside the framework. My biggest concern is addon compatibility, which I don't know enough about to feel confident proposing a migration path.

Given that the entire question is an order of lookup/fallback priority, is this not something that we can make easily configurable in @glimmer/di?

One option would be to have a "load path" like array of resolvers or registries. Lookup could proceed through the array in order, moving to the next resolver/registry if the previous one did not return a result. Ember could then swap the default order of these.

Alternatively, we could establish a protocol for fallback lookups by installing a well-known symbol or property on resolvers/registries. If a resolver/registry didn't return a result, we would use the resolver/registry installed on that property, and so on, until either a result was found or we reached the end of the chain.

I don't think we have time to do a big overhaul of Ember's DI system, so I don't want to propose anything that requires big changes to Ember. But I think this is a rare opportunity to revisit precedence order, and it's something that trips up almost everyone because of how unintuitive it is. If we can find a way to accommodate both constraints, that would be awesome.

@dgeb
Copy link
Member

dgeb commented Apr 19, 2017

@tomdale I have been mulling this over in much the same terms, yet you somehow beat me to posting even while on vacation ;)

One option would be to have a "load path" like array of resolvers or registries.

This is the approach that I was going to propose. It would allow us to remove custom "fallback" logic from the registry, and allow the container to interact with both resolvers and registries through a common shared interface.

I don't think we have time to do a big overhaul of Ember's DI system, so I don't want to propose anything that requires big changes to Ember.

I'm pretty sure that we'd only be adding flexibility at the DI layer, and I think that flexibility could be tuned to keep Ember working "as is".

The TBD portions (in my mind) are options registrations and instance vs. factory registrations (see #21). We should decide on a cohesive proposal to minimize churn. Obviously, I'd like to run any proposals by @mixonic and test out any implementations with his unification work as a sanity check.

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

No branches or pull requests

4 participants