spectator icon indicating copy to clipboard operation
spectator copied to clipboard

Make `ModuleTeardownOptions.destroyAfterEach` default to `true`

Open joepvl opened this issue 3 months ago β€’ 2 comments

Description

The option to pass ModuleTeardownOptions to the createXFactory methods was added in #499 (this was back in 2021). As noted in the discussion on that PR, Currently, Spectator overrides TestBed's default behaviour of setting ModuleTeardownOptions.destroyAfterEach to true. Destroying instances in afterEach has been the default for TestBed since Angular 13. This problem becomes particularly noticeable when using the relatively new takeUntilDestroyed (introduced in Angular 19) operator from @angular/core/rxjs-interop.

Also, Spectator's types refer to this type from @angular/core/testing:

/**
 * Configures the test module teardown behavior in `TestBed`.
 * @publicApi
 */
interface ModuleTeardownOptions {
    /** Whether the test module should be destroyed after every test. Defaults to `true`. */
    destroyAfterEach: boolean;
    /** Whether errors during test module destruction should be re-thrown. Defaults to `true`. */
    rethrowErrors?: boolean;
}

The comment there says "Defaults to true.". Obviously this is a runtime thing that only applies to TestBed, but this does make it look like Spectator does the same thing.

Proposed solution

  1. Make true the new default. (Obviously a breaking change)
  2. Provide a facility to set this globally. (As @NetanelBasal suggested in #499) 1

Alternatives considered

Only update the type definition + documentation to make it clear that the default is false, so at least the behaviour is clear. I don't think we should do this; there's a reason Angular made true the default. It makes it easier to isolate tests and they're more predictable and easier to reason about that way.

Do you want to create a pull request?

Yes, time permitting.

1 Technically this can be done already via defineGlobalsInjections, but it isn't explicitly documented so we can't expect anyone to know about it. Also, if I'm reading this comment correctly, if provided this way the options currently can't be overridden on a per-case basis. That seems undesirable.

joepvl avatar Oct 01 '25 13:10 joepvl

I think we should go with the Angular approach

NetanelBasal avatar Oct 02 '25 17:10 NetanelBasal

I agree! But I do think we need a better API to set the option globally. defineGlobalsInjections doesn't seem like the place for this β€”Β at least IMO its name suggests it's for creating "global providers", like an omnipresent root module. Having the ability to set the ModuleTeardownOptions globally makes sense to me, but conceptually it doesn't have anything to do with what's provided for a given token at any injection site.

joepvl avatar Oct 10 '25 21:10 joepvl