fix(modules): run a module's teardown before invalidating its context - #20
Merged
Merged
Conversation
A module's registrations are released by calling the providing module's unregister functions, through interface facades the core bound to the module that registered them. Those calls are the module's own last work, but `Events.ModuleDestroyed` invalidated its execution context before running any of them, so every facade threw `ModuleContextInvalidatedError` and the registration stayed in the provider for the life of the process — a DMS hot reload left 138 page routes bound to a dead context and every one of them answering 500. Run the async detachments, the register cleanups and `unregisterOwner` first, and invalidate the context once they are done.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Events.ModuleDestroyedinvalidated the module context before running the owner's async cleanups, register cleanups andunregisterOwneron the registering proxies. Any disposer that called an interface function facade bound to that owner threwModuleContextInvalidatedError(assertActiveModuleContext<-runWithCapturedModuleContext), so a module's own un-registration could never run under its own context. In DMS this left every page route registered against a dead context after a dev hot reload (138 teardown warnings,/dms/sitelayout500 forever, no reload event).releaseOwnerRegistrations()now runs first and the context is invalidated in afinallyafterwards. Testteardown-module-context.test.tsreproduces the production failure (red without the fix) and pins that the context is still invalidated once teardown is done.Validation
pnpm lint, format,pnpm build,pnpm test(77),test:package, knip: pass. Verified end to end in the DMS playground with the patched package linked: 0 teardown warnings across two hot reloads.Release: 0.0.13 (patch). Fleet consumers pin
^0.0.12, so their ranges must move to>=0.0.13 <1.0.0to pick it up.