Stop re-evaluating modules that can't see a mock (fix unbounded ESM memory growth) - #122
Merged
Merged
Conversation
Node can't evict modules from its ESM cache, so quibble gets a fresh instance by importing under a new `?__quibble=<generation>` URL. It tagged every import that way, so each new generation re-evaluated (and permanently retained) the whole graph of everything imported while any mock existed, including modules unrelated to the mock. In a suite that imports its subject in a beforeEach this grows without bound. Learn the import graph from the resolve and load hooks instead, and only tag a module that is mocked or can reach a mocked module. A module's imports are only known once it has finished linking, so until it was first loaded in an earlier generation it is assumed to be affected. Unaffected modules are then evaluated about twice (tagged, then at their plain URL) instead of once per generation. Nothing parses source, and dynamic imports need no special handling because they run through the resolve hook when they execute.
The import graph has to record edges even while nothing is mocked: a module loaded then is treated as fully known by the time a mock exists, so skipping its edges would make it look like it depends on nothing and it would never see a mock added later. Nothing covered that, so "fixing" the `!state.quibbledModules` check in planResolve to `.size` (which skips edge recording while nothing is mocked) passed the whole suite while silently breaking this case. Add a test with fixtures that only it uses, so the subject really is first loaded before any mock regardless of test order, and say why in the import-graph header.
`quibbleLoaderState.quibbledModules` is always a Map (reset() replaces it with a new Map rather than clearing it), so `!state.quibbledModules` is never true and the branch never short-circuited anything. Whether anything is mocked is decided later, once the import graph has been recorded.
rosston
marked this pull request as ready for review
October 2, 2026 15:44
This was referenced Oct 2, 2026
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
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.
Fixes #116. Ref testdouble/testdouble.js#534.
The problem
Node can't evict anything from its ESM module cache, so quibble gets a fresh instance of a module by importing it under a new URL: it appends
?__quibble=<generation>, and the generation goes up every time a mock is added or reset. Until now that tag went on every import that resolved while quibble was loaded, whether or not a mock had anything to do with it. Each generation therefore re-evaluated, and permanently retained, the whole graph of everything imported under it, including packages that never touch a mocked module (sinon, chai, lodash, and so on).In a suite that follows testdouble's documented pattern (
td.replaceEsm()and thenawait import(subject)inbeforeEach,td.reset()inafterEach), the subject's whole dependency graph is loaded again for every test and never freed. #116 notes that the!quibbleLoaderState.quibbledModulesguard inresolvecan never be true because it is aMap, which is part of this, but fixing only that guard doesn't help once a mock is registered (see "Alternatives considered"). The heap snapshot in testdouble/testdouble.js#534 shows exactly this shape: one module (app_validation.ts) retained under?__quibble=1,3,5, …37, with the generation going up by two per test (onereplaceEsm, onereset).The change
Only a module that is mocked, or that can reach a mocked module through its imports, needs a fresh URL. This PR teaches the resolve and load hooks to learn those imports as modules load (new
lib/import-graph.js, shared by the async hooks inquibble.mjsand the sync hooks inquibble-sync-hooks.js) and to leave every other module at its plain URL so it stays in Node's cache.import()needs no special handling: it goes through the resolve hook when it runs, so it sees whichever mocks exist at that moment.Measurements
The numbers below are
heapUsedafter a forced GC, from a synthetic repro rather than a real suite. Each iteration isreplaceEsmof a small module, import of a subject,td.reset(), with the subject (or something imported after the reset) pulling in a 150-module graph of roughly 340MB. Run in Docker, quibble 0.10.1 as the baseline and the tip of this branch for the "after" column.td.reset()never calledts-node/esmBefore the change memory grew by about 337MB per iteration on every Node version in testdouble's CI matrix (16, 18, 20, 22, 24, 26), under both
--loader=quibbleand auto-registration. After the change it plateaus at about twice the size of the unaffected graph on the versions and modes I measured (18, 22 and 26, plus thets-node/esmrepro on the same three). The 679MB is the "about twice" described above, not a leak: it stays there for as long as I kept iterating.Behavior change worth noting
A module that is not affected by any mock is now a single cached instance across tests, where it used to get a fresh instance after each
reset()as a side effect of the tagging. That is closer to how plain ESM behaves, but a test that relied on module-level state in an unrelated module being reset between tests would notice. Modules that are mocked or depend on a mock are still re-evaluated against each new mock, which is what the testdouble docs promise.Alternatives considered
!state.quibbledModules?.size) only skips tagging while no mock is registered. It does fix the "imported after reset" shape, but not the documented pattern where the subject is imported while a mock is active, so a suite following the docs still leaks. It would also be actively harmful on top of this change: the early return skips recording imports, so a module imported before any mock exists would look like it depends on nothing and never see a mock added later. The late-mock test (second commit) covers exactly that, and the dead check is removed instead (see last commit).es-module-lexer) gave flat memory at one copy. I didn't ship it because it reads files from disk rather than what the loader actually served, and the maintainedes-module-lexerline has no CommonJS build, so supporting Node 16 and 18 would mean pinning an old major version.vitestandjestavoid the growth by owning their module cache entirely, which is a completely different design decision fromquibbleandtestdouble.js.node:test'smock.moduleavoids the growth by only versioning the mocked module's own URL. As a result an already-imported subject keeps seeing the first mock (I confirmed this on Node 24 and 26), which is a different contract from what testdouble promises.Tests
test/esm-lib/quibble-esm-import-graph.test.mjs(with fixtures intest/esm-fixtures/import-graph/,import-graph-late-mock/,import-graph-stub-first/andimport-graph-missing-module/) has eight tests. They cover a mock reached only transitively, each new mock being picked up, the real module returning afterreset(), mocking a module in the middle of a graph, unrelated modules no longer being re-evaluated for every mock, a module imported before any mock exists still seeing a mock added later, a module that was only ever loaded as a stub not hiding later mocks of its dependencies, and a mock of a module that does not exist being picked up by each of four generations. The unrelated-modules test fails onmain("evaluated 8 times") and passes here. The last three use fixtures nothing else touches, so what each module has been used for beforehand doesn't depend on test order. Each of them fails if the thing it guards is broken: the late-mock test if the removed!state.quibbledModulescheck were restored as.size, the stub-first test without the load fix ('leaf fake'instead of'real'), and the missing-module test without the recovery fix ('second'instead of'third'). None of those three failures were caught by the earlier tests.Not covered
import()s a mocked ESM module is handled by the dynamic-import rule above, but I didn't add a test for that exact shape.Promise.allof imports overlapping areplaceEsm) are a known limitation, not tested. If the generation changes while a module's imports are still resolving, that module can be treated as fully known with only some of its edges, and stay at its plain URL. The missing edges are recorded once those imports resolve, but a decision already made on the partial set stands for that generation, and the instance created from it isn't replaced.a.js?v=1anda.js?v=2share one tagging decision. This is noted in the code and I expect it to be rare.