Change loader module cache to use canonical RRI

packages/host/app/services/loader-service.ts

@@ -46,7 +46,10 @@ export default class LoaderService extends Service {
       // clears the fetch cache and SSR-injected scoped styles in between tests
       this.resetState();
     }
-    registerDestructor(this, () => this.resetState());
+    registerDestructor(this, () => {
+      this.resetState();
+      this.loader?.dispose();
+    });
   }
 
   public resetState() {
@@ -57,9 +60,9 @@ export default class LoaderService extends Service {
     this.resetTime = undefined;
     log.debug(`resetting loader for session boundary (${reason ?? ''})`);
     this.clearSessionCaches();
-    this.loader = this.loader
-      ? Loader.cloneLoader(this.loader)
-      : this.makeInstance();
+    let previous = this.loader;
+    this.loader = previous ? Loader.cloneLoader(previous) : this.makeInstance();
+    previous?.dispose();
   }
 
   public resetLoader(options?: { clearFetchCache?: boolean; reason?: string }) {
@@ -71,6 +74,7 @@ export default class LoaderService extends Service {
       this.resetTime = Date.now();
       log.debug(`resetting loader (clearFetchCache, ${options.reason ?? ''})`);
       clearFetchCache();
+      this.loader?.dispose();
       this.loader = this.makeInstance();
       return;
     }
@@ -85,8 +89,10 @@ export default class LoaderService extends Service {
       log.debug(`resetting loader (${options?.reason ?? ''})`);
       // by default we keep the fetch cache so we can take advantage of HTTP
       // caching when rebuilding the loader state
-      if (this.loader) {
-        this.loader = Loader.cloneLoader(this.loader);
+      let previous = this.loader;
+      if (previous) {
+        this.loader = Loader.cloneLoader(previous);
+        previous.dispose();
       } else {
         this.loader = this.makeInstance();
       }

packages/host/tests/unit/loader-test.ts

@@ -266,6 +266,61 @@ module('Unit | loader', function (hooks) {
     }
   });
 
+  test('a realm-mapping change discards the module cache', async function (assert) {
+    // The loader keys its module cache by canonical RRI form, whose
+    // relationship to a URL is only stable between mapping changes. A mapping
+    // add/remove must therefore discard the cache so an entry can't survive
+    // under a spelling that no longer resolves the same way.
+    let virtualNetwork = getService('network').virtualNetwork;
+    await loader.import(`${testRealmURL}f`);
+    assert.true(
+      loader.isModuleLoaded(`${testRealmURL}f`),
+      'module is cached after import',
+    );
+    virtualNetwork.addRealmMapping('@test-loader-discard/', testRealmURL);
+    try {
+      assert.false(
+        loader.isModuleLoaded(`${testRealmURL}f`),
+        'adding a realm mapping discards the module cache',
+      );
+    } finally {
+      virtualNetwork.removeRealmMapping('@test-loader-discard/');
+    }
+    assert.false(
+      loader.isModuleLoaded(`${testRealmURL}f`),
+      'removing a realm mapping also discards the module cache',
+    );
+  });
+
+  test('dispose() unsubscribes a discarded loader from mapping-change notifications', async function (assert) {
+    // LoaderService replaces its loader on every module edit / session
+    // boundary. Each loader subscribes to realm-mapping changes; without
+    // dispose() the VirtualNetwork keeps that subscription — pinning the
+    // loader and its whole module cache indefinitely. After dispose(), a
+    // mapping change must no longer reach this loader.
+    //
+    // Probe with a net-zero add+remove rather than isModuleLoaded straight
+    // after an add: a mapping add shifts the cache-key form itself, so a
+    // still-cached module would fail to look up under its original URL even
+    // when nothing was discarded. Restoring the mapping restores the key
+    // form, so a surviving cache entry is observable again — which it is only
+    // if the loader ignored both notifications.
+    let virtualNetwork = getService('network').virtualNetwork;
+    let discarded = Loader.cloneLoader(loader);
+    await discarded.import(`${testRealmURL}f`);
+    assert.true(
+      discarded.isModuleLoaded(`${testRealmURL}f`),
+      'module is cached after import',
+    );
+    discarded.dispose();
+    virtualNetwork.addRealmMapping('@test-loader-dispose/', testRealmURL);
+    virtualNetwork.removeRealmMapping('@test-loader-dispose/');
+    assert.true(
+      discarded.isModuleLoaded(`${testRealmURL}f`),
+      "a disposed loader's cache survives mapping changes (subscription released)",
+    );
+  });
+
   test('isModuleLoaded returns false for a module that has not been imported', function (assert) {
     assert.false(
       loader.isModuleLoaded(`${testRealmURL}a`),

packages/runtime-common/loader.ts

@@ -210,6 +210,13 @@ export class Loader {
   private fetchImplementation: Fetch;
   private resolveImport: (moduleIdentifier: string) => string;
   private virtualNetwork: VirtualNetwork | undefined;
+  // Unsubscribe for the realm-mapping-change listener registered below. The
+  // VirtualNetwork outlives any single loader (LoaderService replaces the
+  // loader on every module edit / session boundary), so a loader that isn't
+  // unsubscribed when it's discarded stays pinned — along with its whole
+  // module cache — by the listener the network still holds. `dispose()`
+  // releases it; the owner calls that before dropping the loader.
+  private unsubscribeMappingChange: (() => void) | undefined;
   // When the host runs inside a prerender, `setTimeout` is suppressed by
   // the render-timer-stub so the default sleep used by
   // `fetchWithTransientRetry` would never resolve and a transient 5xx on
@@ -231,6 +238,23 @@ export class Loader {
       resolveImport ?? ((moduleIdentifier) => moduleIdentifier);
     this.retrySleep = options?.retrySleep;
     this.virtualNetwork = options?.virtualNetwork;
+    // Module caches are keyed by canonical RRI form (see moduleCacheKey), whose
+    // relationship to a real URL is only stable between realm-mapping changes.
+    // Discard the RRI-keyed caches whenever a mapping is added or removed so an
+    // entry can't outlive the spelling it was keyed under.
+    this.unsubscribeMappingChange = this.virtualNetwork?.onMappingChange(() => {
+      this.modules.clear();
+      this.moduleCanonicalURLs.clear();
+      this.knownDepsCache.clear();
+    });
+  }
+
+  // Release the realm-mapping-change subscription so this loader can be
+  // garbage-collected once discarded. Only detaches the listener — it does not
+  // clear the caches, since a clone made from this loader carries them forward.
+  dispose() {
+    this.unsubscribeMappingChange?.();
+    this.unsubscribeMappingChange = undefined;
   }
 
   getVirtualNetwork(): VirtualNetwork | undefined {
@@ -833,36 +857,23 @@ export class Loader {
     }
   };
 
-  // Cache key for the per-module maps. Collapses the virtual-alias URL and the
-  // resolved real URL of the same module onto one key, so a base module
-  // imported via the alias (`https://cardstack.com/base/X`) and via the RRI
-  // prefix (`@cardstack/base/X` → resolveImport → resolved real URL) share one
-  // cached module — and therefore one class object. Without this, the alias
-  // and RRI forms evaluate as two distinct modules and `instanceof` /
-  // polymorphic-field identity checks across them diverge. Mirrors the
-  // real→virtual convention used by `canonicalizeTrackingKey`.
+  // Cache key for the per-module maps: the canonical RRI form. Every spelling
+  // of a module — its resolved real URL, the virtual-alias URL, and the RRI
+  // prefix — folds to one RRI via `unresolveURL`, so a base module reached by
+  // any of them shares one cached module and therefore one class object.
+  // Without this collapse the spellings evaluate as distinct modules and
+  // `instanceof` / polymorphic-field identity checks across them diverge.
+  // Modules with no realm-prefix mapping (user realms, bare package specifiers)
+  // are returned unchanged by `unresolveURL`.
   //
-  // The key deliberately does NOT use the realm-prefix (RRI) form even
-  // though that's the canonical form for identifiers flowing out of the
-  // loader: realm-prefix mappings can be registered and removed while a
-  // loader is live (tests scope temporary prefixes via
-  // `addRealmMapping`/`removeRealmMapping`), and a mapping-sensitive key
-  // would orphan already-cached entries whenever a mapping changes —
-  // re-evaluating the same module under a new key and splitting class
-  // identities across the old and new copies.
+  // The RRI→URL relationship is only stable between realm-mapping changes, so
+  // the caches keyed here are discarded whenever a mapping is added or removed
+  // (see the `onMappingChange` subscription in the constructor).
   private moduleCacheKey(moduleIdentifier: string): string {
     let trimmed = trimModuleIdentifier(moduleIdentifier);
-    if (this.virtualNetwork) {
-      try {
-        let virtual = this.virtualNetwork.mapURL(trimmed, 'real-to-virtual');
-        if (virtual) {
-          return virtual.href;
-        }
-      } catch {
-        // not a parseable URL (e.g. a bare specifier) — fall through
-      }
-    }
-    return trimmed;
+    return this.virtualNetwork
+      ? this.virtualNetwork.unresolveURL(trimmed)
+      : trimmed;
   }
 
   private getModule(moduleIdentifier: string): Module | undefined {

packages/runtime-common/virtual-network.ts

@@ -35,11 +35,29 @@ export class VirtualNetwork {
   // mapping is added or removed (both clear the cache).
   private toURLHrefCache = new Map<string, string>();
 
+  // Notified whenever a realm-prefix mapping is added or removed. Consumers
+  // that key caches by the RRI form a mapping produces (e.g. the Loader's
+  // module cache) subscribe here to discard those entries when the mapping
+  // set changes — the RRI→URL relationship is only stable between changes.
+  private mappingChangeListeners = new Set<() => void>();
+
   constructor(nativeFetch = createEnvironmentAwareFetch()) {
     this.nativeFetch = nativeFetch;
     this.mount(this.packageShimHandler.handle);
   }
 
+  // Subscribe to realm-mapping changes; returns an unsubscribe function.
+  onMappingChange(listener: () => void): () => void {
+    this.mappingChangeListeners.add(listener);
+    return () => this.mappingChangeListeners.delete(listener);
+  }
+
+  private notifyMappingChange() {
+    for (let listener of this.mappingChangeListeners) {
+      listener();
+    }
+  }
+
   resolveImport = (moduleIdentifier: string) => {
     for (let [prefix, handler] of this.importMap) {
       if (moduleIdentifier.startsWith(prefix)) {
@@ -98,6 +116,7 @@ export class VirtualNetwork {
       normalizedId,
       (rest) => new URL(rest, normalizedTarget).href,
     );
+    this.notifyMappingChange();
   }
 
   /**
@@ -111,6 +130,7 @@ export class VirtualNetwork {
     this.realmMappings.delete(normalizedId);
     this.importMap.delete(normalizedId);
     this.toURLHrefCache.clear();
+    this.notifyMappingChange();
   }
 
   knownRealms(): RealmIdentifier[] {