From e987bbdc2c7d399f84eaf8da88b02f31205b0b12 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Wed, 25 Mar 2026 15:35:38 -0400 Subject: [PATCH 1/3] Don't refresh loader when a new gts file is added if the module has never been loaded When a new executable file (.gts/.ts/.js/.gjs) is written to a realm, skip the expensive loader reset if the module was never imported. This prevents unnecessary full UI refreshes in notebook-style workflows where AI writes new code files sequentially. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/host/app/resources/file.ts | 5 +- packages/host/app/services/card-service.ts | 5 +- packages/host/app/services/store.ts | 15 +++-- packages/host/tests/unit/loader-test.ts | 78 ++++++++++++++++++++++ packages/runtime-common/loader.ts | 6 ++ 5 files changed, 103 insertions(+), 6 deletions(-) diff --git a/packages/host/app/resources/file.ts b/packages/host/app/resources/file.ts index c7f0415d724..8a5f9a4ea1f 100644 --- a/packages/host/app/resources/file.ts +++ b/packages/host/app/resources/file.ts @@ -343,7 +343,10 @@ class _FileResource extends Resource { clientRequestId: opts?.clientRequestId, }, ); - if (opts?.flushLoader) { + if ( + opts?.flushLoader && + this.loaderService.loader.isModuleLoaded(this._url) + ) { this.store.refreshReferencesForCodeChange('file write'); } if (this.innerState.state === 'not-found') { diff --git a/packages/host/app/services/card-service.ts b/packages/host/app/services/card-service.ts index 8038782ba4a..91c445ef690 100644 --- a/packages/host/app/services/card-service.ts +++ b/packages/host/app/services/card-service.ts @@ -239,7 +239,10 @@ export default class CardService extends Service { } this.subscriber?.(url, content); - if (options?.resetLoader) { + if ( + options?.resetLoader && + this.loaderService.loader.isModuleLoaded(url.href) + ) { this.loaderService.resetLoader(); } diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index 2b08c526fc2..e7e628ce70f 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -1060,10 +1060,17 @@ export default class StoreService extends Service implements StoreInterface { } let invalidations = event.invalidations as string[]; - if (invalidations.find((i) => hasExecutableExtension(i))) { - // the invalidation included code changes too. in this case we - // need to flush the loader so that we can pick up any updated - // code before re-running the card + if ( + invalidations.find( + (i) => + hasExecutableExtension(i) && + this.loaderService.loader.isModuleLoaded(i), + ) + ) { + // the invalidation included code changes to modules that are already + // loaded. in this case we need to flush the loader so that we can pick + // up the updated code before re-running the card. net-new modules that + // have never been loaded don't require a loader reset. this.loaderService.resetLoader(); this.store.reset(); this.reestablishReferences.perform(); diff --git a/packages/host/tests/unit/loader-test.ts b/packages/host/tests/unit/loader-test.ts index 36decb917d7..0f120481704 100644 --- a/packages/host/tests/unit/loader-test.ts +++ b/packages/host/tests/unit/loader-test.ts @@ -126,6 +126,9 @@ module('Unit | loader', function (hooks) { 'foo.js': ` export function checkImportMeta() { return import.meta.url; } export function myLoader() { return import.meta.loader; } + `, + 'reexporter.js': ` + export { g } from './g'; `, }, }), @@ -254,6 +257,81 @@ module('Unit | loader', function (hooks) { ); }); + test('isModuleLoaded returns false for a module that has not been imported', function (assert) { + assert.false( + loader.isModuleLoaded(`${testRealmURL}a`), + 'module a is not loaded before import', + ); + assert.false( + loader.isModuleLoaded(`${testRealmURL}nonexistent`), + 'nonexistent module is not loaded', + ); + }); + + test('isModuleLoaded returns true for a module that has been imported', async function (assert) { + assert.false( + loader.isModuleLoaded(`${testRealmURL}a`), + 'module a is not loaded before import', + ); + await loader.import(`${testRealmURL}a`); + assert.true( + loader.isModuleLoaded(`${testRealmURL}a`), + 'module a is loaded after import', + ); + }); + + test('isModuleLoaded returns true for dependencies of an imported module', async function (assert) { + assert.false( + loader.isModuleLoaded(`${testRealmURL}b`), + 'module b is not loaded before import', + ); + assert.false( + loader.isModuleLoaded(`${testRealmURL}c`), + 'module c is not loaded before import', + ); + await loader.import(`${testRealmURL}a`); + assert.true( + loader.isModuleLoaded(`${testRealmURL}b`), + 'module b is loaded as a dependency of a', + ); + assert.true( + loader.isModuleLoaded(`${testRealmURL}c`), + 'module c is loaded as a transitive dependency of a', + ); + }); + + test('isModuleLoaded works with executable extensions in the URL', async function (assert) { + await loader.import(`${testRealmURL}person`); + assert.true( + loader.isModuleLoaded(`${testRealmURL}person`), + 'loaded without extension', + ); + assert.true( + loader.isModuleLoaded(`${testRealmURL}person.gts`), + 'loaded with .gts extension', + ); + }); + + test('isModuleLoaded returns true for a re-exported module', async function (assert) { + assert.false( + loader.isModuleLoaded(`${testRealmURL}reexporter`), + 'reexporter is not loaded before import', + ); + assert.false( + loader.isModuleLoaded(`${testRealmURL}g`), + 're-exported module g is not loaded before import', + ); + await loader.import(`${testRealmURL}reexporter`); + assert.true( + loader.isModuleLoaded(`${testRealmURL}reexporter`), + 'reexporter is loaded after import', + ); + assert.true( + loader.isModuleLoaded(`${testRealmURL}g`), + 're-exported module g is loaded as a dependency of reexporter', + ); + }); + test('identify preserves original module for reexports', function (assert) { let throwIfFetch = new Loader(async () => { throw new Error( diff --git a/packages/runtime-common/loader.ts b/packages/runtime-common/loader.ts index fab0338d919..ff35dc910dd 100644 --- a/packages/runtime-common/loader.ts +++ b/packages/runtime-common/loader.ts @@ -266,6 +266,12 @@ export class Loader { } } + isModuleLoaded(moduleIdentifier: string): boolean { + moduleIdentifier = this.resolveImport(moduleIdentifier); + let resolvedModuleIdentifier = new URL(moduleIdentifier).href; + return this.getModule(resolvedModuleIdentifier) !== undefined; + } + getKnownConsumedModules(moduleIdentifier: string): string[] { let resolvedModuleIdentifier = this.resolveImport(moduleIdentifier); let knownDependencies = this.collectKnownModuleDependencies( From 03a56cbaaf66b7ab955a2b5416457dde54ce9b85 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Wed, 25 Mar 2026 15:57:28 -0400 Subject: [PATCH 2/3] Address PR review: fix timing issue and add URL safety - Fix file.ts: capture isModuleLoaded BEFORE saveSource (which may reset the loader), so the check sees the pre-reset loader state - Fix store.ts: use wasModuleLoaded() which checks both current and previous loader, handling the timing window where the save path resets the loader before the realm invalidation event arrives - Add previousLoader tracking to LoaderService so module-loaded checks survive across loader resets within the same invalidation cycle - Add try/catch to isModuleLoaded for non-URL identifiers Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/host/app/resources/file.ts | 10 ++++++---- packages/host/app/services/loader-service.ts | 17 +++++++++++++++++ packages/host/app/services/store.ts | 2 +- packages/runtime-common/loader.ts | 13 ++++++++++--- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/host/app/resources/file.ts b/packages/host/app/resources/file.ts index 8a5f9a4ea1f..eea0862ee60 100644 --- a/packages/host/app/resources/file.ts +++ b/packages/host/app/resources/file.ts @@ -334,6 +334,11 @@ class _FileResource extends Resource { clientRequestId?: string; }, ) => { + // Capture before saveSource which may call resetLoader(), replacing + // the loader with a fresh clone that has no loaded modules. + let moduleWasLoaded = + opts?.flushLoader && + this.loaderService.loader.isModuleLoaded(this._url); let response = await this.cardService.saveSource( new URL(this._url), content, @@ -343,10 +348,7 @@ class _FileResource extends Resource { clientRequestId: opts?.clientRequestId, }, ); - if ( - opts?.flushLoader && - this.loaderService.loader.isModuleLoaded(this._url) - ) { + if (moduleWasLoaded) { this.store.refreshReferencesForCodeChange('file write'); } if (this.innerState.state === 'not-found') { diff --git a/packages/host/app/services/loader-service.ts b/packages/host/app/services/loader-service.ts index 14ff850fc94..338c80f6eb0 100644 --- a/packages/host/app/services/loader-service.ts +++ b/packages/host/app/services/loader-service.ts @@ -36,6 +36,7 @@ export default class LoaderService extends Service { @service declare private reset: ResetService; @tracked public loader = this.makeInstance(); + private previousLoader: Loader | null = null; private resetTime: number | undefined; constructor(owner: Owner) { @@ -47,11 +48,13 @@ export default class LoaderService extends Service { } public resetState() { + this.previousLoader = null; this.clearSessionCaches(); } public resetSessionBoundary(reason?: string) { this.resetTime = undefined; + this.previousLoader = null; log.debug(`resetting loader for session boundary (${reason ?? ''})`); this.clearSessionCaches(); this.loader = this.loader @@ -67,6 +70,7 @@ export default class LoaderService extends Service { if (options?.clearFetchCache) { this.resetTime = Date.now(); log.debug(`resetting loader (clearFetchCache, ${options.reason ?? ''})`); + this.previousLoader = this.loader; clearFetchCache(); this.loader = this.makeInstance(); return; @@ -80,6 +84,7 @@ export default class LoaderService extends Service { if (this.resetTime == null || Date.now() - this.resetTime > 250) { this.resetTime = Date.now(); log.debug(`resetting loader (${options?.reason ?? ''})`); + this.previousLoader = this.loader; // by default we keep the fetch cache so we can take advantage of HTTP // caching when rebuilding the loader state if (this.loader) { @@ -90,6 +95,18 @@ export default class LoaderService extends Service { } } + // Check if a module was loaded in the current loader or in the loader that + // was active before the most recent reset. This handles the timing window + // where a save path resets the loader before the realm invalidation event + // arrives: the module is no longer in the new (cloned) loader, but it was + // in the previous one. + public wasModuleLoaded(moduleIdentifier: string): boolean { + return ( + this.loader.isModuleLoaded(moduleIdentifier) || + (this.previousLoader?.isModuleLoaded(moduleIdentifier) ?? false) + ); + } + private makeInstance() { let middlewareStack: FetcherMiddlewareHandler[] = []; middlewareStack.push(async (req, next) => { diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index e7e628ce70f..63443468a30 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -1064,7 +1064,7 @@ export default class StoreService extends Service implements StoreInterface { invalidations.find( (i) => hasExecutableExtension(i) && - this.loaderService.loader.isModuleLoaded(i), + this.loaderService.wasModuleLoaded(i), ) ) { // the invalidation included code changes to modules that are already diff --git a/packages/runtime-common/loader.ts b/packages/runtime-common/loader.ts index ff35dc910dd..99504a1d962 100644 --- a/packages/runtime-common/loader.ts +++ b/packages/runtime-common/loader.ts @@ -267,9 +267,16 @@ export class Loader { } isModuleLoaded(moduleIdentifier: string): boolean { - moduleIdentifier = this.resolveImport(moduleIdentifier); - let resolvedModuleIdentifier = new URL(moduleIdentifier).href; - return this.getModule(resolvedModuleIdentifier) !== undefined; + try { + moduleIdentifier = this.resolveImport(moduleIdentifier); + let resolvedModuleIdentifier = new URL(moduleIdentifier).href; + return this.getModule(resolvedModuleIdentifier) !== undefined; + } catch (e) { + if (e instanceof TypeError) { + return false; + } + throw e; + } } getKnownConsumedModules(moduleIdentifier: string): string[] { From b8c147b88e2f933ac1ab1d4a92d17b6f81f1c6c2 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Wed, 25 Mar 2026 16:15:02 -0400 Subject: [PATCH 3/3] Remove unnecessary previousLoader tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the current loader has never encountered a module, no card was rendered using it through this loader — there's nothing stale to refresh. When the save path already called resetLoader(), the tracked property change on the loader triggers Ember's reactivity, causing re-renders that use the new loader to import updated code fresh. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/host/app/services/loader-service.ts | 17 ----------------- packages/host/app/services/store.ts | 2 +- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/packages/host/app/services/loader-service.ts b/packages/host/app/services/loader-service.ts index 338c80f6eb0..14ff850fc94 100644 --- a/packages/host/app/services/loader-service.ts +++ b/packages/host/app/services/loader-service.ts @@ -36,7 +36,6 @@ export default class LoaderService extends Service { @service declare private reset: ResetService; @tracked public loader = this.makeInstance(); - private previousLoader: Loader | null = null; private resetTime: number | undefined; constructor(owner: Owner) { @@ -48,13 +47,11 @@ export default class LoaderService extends Service { } public resetState() { - this.previousLoader = null; this.clearSessionCaches(); } public resetSessionBoundary(reason?: string) { this.resetTime = undefined; - this.previousLoader = null; log.debug(`resetting loader for session boundary (${reason ?? ''})`); this.clearSessionCaches(); this.loader = this.loader @@ -70,7 +67,6 @@ export default class LoaderService extends Service { if (options?.clearFetchCache) { this.resetTime = Date.now(); log.debug(`resetting loader (clearFetchCache, ${options.reason ?? ''})`); - this.previousLoader = this.loader; clearFetchCache(); this.loader = this.makeInstance(); return; @@ -84,7 +80,6 @@ export default class LoaderService extends Service { if (this.resetTime == null || Date.now() - this.resetTime > 250) { this.resetTime = Date.now(); log.debug(`resetting loader (${options?.reason ?? ''})`); - this.previousLoader = this.loader; // by default we keep the fetch cache so we can take advantage of HTTP // caching when rebuilding the loader state if (this.loader) { @@ -95,18 +90,6 @@ export default class LoaderService extends Service { } } - // Check if a module was loaded in the current loader or in the loader that - // was active before the most recent reset. This handles the timing window - // where a save path resets the loader before the realm invalidation event - // arrives: the module is no longer in the new (cloned) loader, but it was - // in the previous one. - public wasModuleLoaded(moduleIdentifier: string): boolean { - return ( - this.loader.isModuleLoaded(moduleIdentifier) || - (this.previousLoader?.isModuleLoaded(moduleIdentifier) ?? false) - ); - } - private makeInstance() { let middlewareStack: FetcherMiddlewareHandler[] = []; middlewareStack.push(async (req, next) => { diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index 63443468a30..e7e628ce70f 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -1064,7 +1064,7 @@ export default class StoreService extends Service implements StoreInterface { invalidations.find( (i) => hasExecutableExtension(i) && - this.loaderService.wasModuleLoaded(i), + this.loaderService.loader.isModuleLoaded(i), ) ) { // the invalidation included code changes to modules that are already