CrowdStrike / ember-browser-services

Services for interacting with browser APIs so that you can have fine-grained control in tests.
MIT License
47 stars 12 forks source link

setupBrowserFakes doesn't work if your service injections are namespaced #420

Open elwayman02 opened 2 days ago

elwayman02 commented 2 days ago

In the current Ember world where we don't yet have a specific pattern for importing a service class and injecting it directly, it's a common practice to properly namespace service injections so you know where they're coming from. Since many addons provide services that may have duplicate names, this is the only way to ensure you're using the service instance you expected to inject.

However, the way setupBrowserFakes is written doesn't account for this possibility, causing test mocking to fail.

Given the following service injection:

import { type NavigatorService } from 'ember-browser-services/types';

// ...

@service('ember-browser-services@browser/navigator')
declare navigator: NavigatorService;

setupBrowserFakes will not correctly replace the proxy values, because it registers the browser fakes to browser/navigator rather than ember-browser-services@browser/navigator (and so on for the other services).

This can be seen here: https://github.com/CrowdStrike/ember-browser-services/blob/main/ember-browser-services/src/test-support/index.ts#L39-L66

We need to devise a way to support both injection patterns within this addon's test-support.

NullVoxPopuli commented 2 days ago

it's a common practice to properly namespace service injections

via @? is that supported by ember? (like, would it work in embroider/webpack/vite?) are there docs or an RFC on this?

ef4 commented 1 day ago

The @ doesn't work under embroider with staticAddonTrees enabled (which will be a default in the next major). I wouldn't recommend anybody use it.

I appreciate the desire to make it clearer where a service is coming from but that needs to happen via a real import, like in ember-polaris-service. Otherwise you're just making the "Ember apps are hard to build and optimize because they don't use real imports" problem worse.