Race condition for setupBrowserFakes vs. first lookup of service

[simonihmig]

This addon has a flaw that I don't know how to best fix (without major changes). The problem is that opting into the mocked behavior is done by calling setupBrowserFakes() in tests, which will register the service (e.g. service:browser/window) with a mockable Proxy class here. However, owner.register() will only work when that service hasn't been used before. So that opens up the possibility to run into race conditions when service:browser/window has been looked up (e.g. by having an @service('browser/window') field be evaluated during app boot) before setupBrowserFakes() was called.

In that case, setupBrowserFakes(hooks, { window: { location: { href: 'https://example.com' } } }) would have no effect. The service would be the one directly proxying to the actual window global, not the Proxy from ember-window-mock. As such this.owner.lookup('service:browser/window').location.href would not be 'https://example.com' (but http://localhost:4200/tests or similar), and this.owner.lookup('service:browser/window').location.href = 'https://example.com'; would actually do a real redirect.

[NullVoxPopuli]

🤔 what causes the app to boot and how is it happening before setupBrowserFakes?

[simonihmig]

One instance where this came up was rooted in an initializer running (apparently so before setupBrowserFakes), that would lookup a service, that has a connection (via @service) to the window service.

[NullVoxPopuli]

ah, yeah -- initializers have proven to often be problematic -- because while it was browser-services hit this time, it more often affects ones own codebase / services -- or maybe a different addon's services (ember-intl's service is a common one also hit by this initializer behavior).

If possible, I think folks should move all their initializer logic to the application route and init there -- which also gets you async.

[simonihmig]

I do wonder though why we need that timing-sensitive register behaviour. After all, ember-window-mock allows you to opt into mocked/proxy mode at any time. Can't the service just use that directly, like all the time?

[NullVoxPopuli]

Can't the service just use that directly, like all the time?

yeah, I don't see a reason why it wouldn't be able to.
Perhaps the original thinking was to "be as light as possible". Maybe it went too far.