fix(modules): run a module's teardown before invalidating its context

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);
+  }
 });
 
 /**

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<string>();
+    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<typeof captureModuleContext>;
+    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/);
+  });
+});