diff --git a/src/modules.ts b/src/modules.ts index 7d9be4d..7f07559 100644 --- a/src/modules.ts +++ b/src/modules.ts @@ -104,9 +104,18 @@ function getDestroyedOwner(module: string): string { return context.owner ?? module; } -Events.ModuleDestroyed.register((module) => { - const owner = getDestroyedOwner(module); - invalidateModuleContext(owner); +/** + * Takes a destroyed module's registrations back down. + * + * Runs while the owner's context is still valid: un-registering is the + * module's own last piece of work, and the disposers doing it legitimately + * call interface functions bound to that context — a route registered through + * `api` is released by calling `api`'s unregister function, through a facade + * captured when the module registered it. Invalidating first turned every such + * call into a `ModuleContextInvalidatedError` and left the registration + * standing in the providing module for as long as the process lived. + */ +function releaseOwnerRegistrations(module: string, owner: string) { for (const cleanup of internal.knownAsync.get(owner) ?? []) { runCleanup(cleanup, owner, "detach-async-provider"); } @@ -135,6 +144,16 @@ Events.ModuleDestroyed.register((module) => { }); } } +} + +Events.ModuleDestroyed.register((module) => { + const owner = getDestroyedOwner(module); + try { + releaseOwnerRegistrations(module, owner); + } finally { + // Only now: nothing else may claim to act for this generation. + invalidateModuleContext(owner); + } }); /** diff --git a/src/tests/teardown-module-context.test.ts b/src/tests/teardown-module-context.test.ts new file mode 100644 index 0000000..af09c59 --- /dev/null +++ b/src/tests/teardown-module-context.test.ts @@ -0,0 +1,86 @@ +import { expect } from "chai"; + +import { RegisteringProxy } from ".."; +import { Events, RunWithModuleContext } from "../modules"; +import { + captureModuleContext, + internal, + runWithCapturedModuleContext, +} from "../internal"; + +describe("teardown under the destroyed module's own context", () => { + let reported: unknown[]; + + beforeEach(() => { + reported = []; + internal.runtimeErrorReporter = (error) => reported.push(error); + }); + + afterEach(() => { + internal.runtimeErrorReporter = undefined; + }); + + it("lets a register cleanup call an interface function bound to that module", () => { + // The shape that broke DMS hot reload: a page registered with one module + // is taken down by calling another module's unregister function, through + // a facade the core bound to the module that did the registering. That + // call is the dying module's own last work — invalidating its context + // before running it turned every teardown step into an error and left the + // route registered in the provider for the life of the process. + const pages = new RegisteringProxy<(id: string) => void>( + "teardown.RegisterPage", + ); + const routes = new Set(); + let unregisterRoute: ((id: string) => void) | undefined; + + RunWithModuleContext( + { module: "dms", owner: "dms#1", provider: "dms" }, + () => + pages.onHandlers( + (id) => { + routes.add(id); + }, + (id) => unregisterRoute?.(id), + ), + ); + + RunWithModuleContext( + { module: "playground", owner: "playground#1" }, + () => { + // What the resolver hands a consumer: the provider's function pinned to + // the context that imported it. + const captured = captureModuleContext(); + expect(captured).to.not.equal(undefined); + unregisterRoute = (id) => + runWithCapturedModuleContext(captured!, () => { + routes.delete(id); + }); + pages.register("/modules/demo/pages/contact"); + }, + ); + + expect([...routes]).to.deep.equal(["/modules/demo/pages/contact"]); + + RunWithModuleContext({ module: "playground", owner: "playground#1" }, () => + Events.ModuleDestroyed.emit("playground"), + ); + + expect(reported).to.deep.equal([]); + expect([...routes]).to.deep.equal([]); + }); + + it("still invalidates the context once teardown is done", () => { + let captured: ReturnType; + RunWithModuleContext({ module: "gone", owner: "gone#1" }, () => { + captured = captureModuleContext(); + }); + + RunWithModuleContext({ module: "gone", owner: "gone#1" }, () => + Events.ModuleDestroyed.emit("gone"), + ); + + expect(() => + runWithCapturedModuleContext(captured!, () => undefined), + ).to.throw(/invalidated/); + }); +});