Re-registration by initializers on App.reset

[trentmwillis]

While looking into using App.reset() for acceptance tests, I stumbled onto this error:

Error: Cannot re-register: `service:-routing`, as it has already been resolved.

It seems the root cause is if you use the routing service (e.g., in a link-to) it gets cached in the application registry and then when you restart, it is picked up by the initializer running again and throws the error. I worked around that particular instance only to have a similar issue crop up with registration of transforms by Ember Data.

I'm wondering if re-running initializers should be part of resetting an application. I feel like semantically, only instance initializer should be re-ran. Thoughts? It seems like we could get some faster test times without having to completely recreate the application each time, but this blocks us from being able to use new instances right now.

Here's a super simple repro case: JSBin

[rwjblue]

I feel like semantically, only instance initializer should be re-ran.

I agree.

[stefanpenner]

app instances and app.reset are two different things.

App.reset is older and is more or less meant to rebuild everything while preserving the namespace. This is to support globals, which used the namespace to store various factories.

In a world of modules the namespace doesn't need to be perserved.

The app.visit + appInstance work exists to replace for both SSR and testing. In the world of appInstances, instance vs non instance initializers actually have meaning.

–––

If you are trying to make tests faster, fixing the appInstance path is going to be the best path forward. As this is actually its intended design.

It is also important that our SSR use-cases, and general app testing are as close as possible. So having testing use the same path, will be net positive.

[rwjblue]

I believe that the acceptance test helpers in ember-testing will need some significant work to make compatible with living on ApplicationInstance instances, but that is likely the best path forward IMHO.

aka, what Stef said 😺

[stefanpenner]

That being said App.reset should work, as it should blow away and rebuild the original registry.
It's worth noting, this likely wont give you the perf you want as that is what the appInstances work is for.

This may have been a regression, @dgeb anything come to mind?

[rwjblue]

@stefanpenner - I have a fix for that bit (basically we need to not be using initializers internally for this) incoming.

[stefanpenner]

@rwjblue 👍

[trentmwillis]

Thanks for the clarification everyone. I'm glad this issue is getting addressed, but will look into the appInstance approach for test perf improvement instead.

@rwjblue do you have any thoughts on how to refactor the test helpers to support using instances? I took a look, and they seem fairly bound to the application itself. I'd be more than happy to pursue whatever work needs to be done to get them working.

[stefanpenner]

@trentmwillis sorry for the upfront confusion, i hope my explanation made sense. I can gladly provide more context if you are interested. I would love to see all our efforts aligned