microsoft / tsyringe

Lightweight dependency injection container for JavaScript/TypeScript
MIT License
5.05k stars 167 forks source link

container.clearInstances() should not clear ValueProviders #121

Open geekflyer opened 4 years ago

geekflyer commented 4 years ago

Describe the bug

One can register dependencies as ValueProvider via:

container.register(TOKEN, {useValue: myImpl});

Unfortunately container.clearInstances() unregisters dependencies registered via ValueProvider, which makes it impossible to resolve TOKEN after calling container.clearInstances().

To Reproduce

import "reflect-metadata"
import { container } from "tsyringe";

const DEP_TOKEN = Symbol("DEP");

const myDepImpl = {
  doStuff() {
    console.log("foo");
  },
} as const;

container.register(DEP_TOKEN, { useValue: myDepImpl });

container.resolve<typeof myDepImpl>(DEP_TOKEN).doStuff(); // prints "foo"

container.clearInstances();

// errors with: Error: Attempted to resolve unregistered dependency token: "Symbol(DEP)"
container.resolve<typeof myDepImpl>(DEP_TOKEN).doStuff();

Expected behavior I would expect container.clearInstances() to only clear instances that were created via a ClassProvider or FactoryProvider but not via ValueProvider as it makes it otherwise impossible / impractical to use .clearInstances() in conjunction with ValueProviders.

Version:

4.3.0

Xapphire13 commented 3 years ago

The clearInstances() method is defined as:

The container.clearInstances() method allows you to clear all previously created and registered instances:

Which means it will clear all instances registered within the container (i.e. Those registered via .registerInstance(...) will be cleared). .registerInstance(...) uses value providers under the hood, so making it behave as you describe would break that functionality.

fknop commented 3 years ago

Do you have a workaround for testing purposes? We're using container.clearInstances() inside the beforeEach as the documentation suggests, however it deletes all "primitive" values as well

WhereJuly commented 1 year ago

All in all lovely tight lean tsyringe, thanks to creators. Enjoying it much.

@fknop , @geekflyer, @Xapphire13

We're using container.clearInstances() inside the beforeEach as the documentation suggests, however it deletes all "primitive" values as well

For this issue in test I do not use clearInstances(). I just restore the initial registration value after I replaced it with mock.

// In application code
@singleton() // Could be any other available instantiation scope
class Foo {
  method() {}
}

// In tests
it('test with mocked Foo', () => {
  // Arrange
  container.registerInstance(Foo, { method: vi.fn().mockImplementation(() => {// just empty mock here}) } as unknown as Foo);
  // Act: do someting that engages Foo behind the scenes via container.resolve()
  // Assert
  const actual = container.resolve(Foo).send as Mock;
  expect(actual.mock.calls.length).toEqual(1);
  expect(actual.mock.calls[0][0]).toBeInstanceOf(...whatever is expected there to be);

  // Restore the normal application Foo instance. 
  // Note to use the scope matching your in-application Foo registration (singleton in this case)
  container.registerSingleton(Foo);
});

Depending on you tests structure you could place this in a single test as in above example or mock / restore Foo in beforeAll(), beforeEach(), afterAll(), afterEach() test suite methods.

I coud prepare a PR with a documentaion update if desirable.

dpobel commented 1 month ago

We've got exactly the same issue and as @geekflyer I find it quite unexpected.