review pass

Author: idudinov

README.md

@@ -19,7 +19,7 @@ All build/test/lint commands live in one place: [`packages/scripts`](./packages/
 - Run tasks from a package dir: `npm run <task>`.
 - Do not call `tsc` / `vitest` / `eslint` directly — the tasks carry the right configs.
 - `npm run dev` from the repo root type-checks every package in one watching `tsc` process — sources, tests, `packages/common/utils` and `packages/scripts` (see [`tsconfig.dev.json`](./tsconfig.dev.json)). It also refreshes `dist/`, since each package's check project references the sibling `dist` projects it depends on.
-- `npm run build:dev` is the same thing without watching — one pass over everything, useful before a commit or in CI.
+- `npm run build:dev` is the same thing without watching — one pass over everything, useful before a commit.
   Reach for it whenever test files changed: `build` compiles sources only, and `test` runs them without type-checking, so a type error in a test slips past both.
 - Args after `--` are passed to the underlying command. Example — run one test:
 

packages/common-mobx/README.md

@@ -13,7 +13,9 @@ All runtime dependencies are peer.
 * [ViewModels]('./src/viewModels/index.ts') – useful for MVVM pattern
 
 * Structures for caching & observing: [`PromiseCacheObservable`](./src/structures/promiseCache.ts) — a thin `PromiseCache` preset wired to a mobx storage provider (also exported standalone as `mobxStorageProvider`, for composing custom presets).
-  `cache.expire(key)` on a settled key wakes no reaction, so a reactive reader keeps serving the current value until something else makes it read; expiring a key with a fetch in flight does notify, since the loading state changes.
+
+* `cache.expire(key)` notifies no observer.
+  A reactive reader keeps serving the current value until something else makes it read.
   Same for `lazy.expireTracker.expire()` on `LazyPromiseObservable`.
 
 * [`TransitionObserver`](./src/observing/transition.ts) – neat wrapper of mobx's `reaction`

packages/common/src/lazy/__tests__/mapped.test.ts

@@ -77,6 +77,19 @@ describe('MappedLazyPromiseView', () => {
         expect(view.hasResolvedValue()).toBe(false);
     });
 
+    test('promise on a failed source resolves to the mapped fallback, not a rejection', async () => {
+        const error = new Error('fail');
+        const source = new LazyPromise<number>(() => setTimeoutAsync(10).then((): number => { throw error; }));
+        const map = vi.fn((v: number | undefined) => v ?? -1);
+        const view = new MappedLazyPromiseView(source, map);
+
+        const p = view.promise;
+        await vi.advanceTimersByTimeAsync(10);
+
+        await expect(p).resolves.toBe(-1);
+        expect(view.hasResolvedValue()).toBe(false);
+    });
+
     test('promise resolves to the mapped value', async () => {
         const source = new LazyPromise(() => setTimeoutAsync(10).then(() => 1));
         const view = new MappedLazyPromiseView(source, v => `n:${v}`);

packages/common/src/lazy/__tests__/promise.expireTracker.test.ts

@@ -60,7 +60,7 @@ describe('LazyPromise.expireTracker', () => {
         expect(lazy.pendingState).toBeNull();
     });
 
-    test('a successful settle clears staleness — no second refetch', async () => {
+    test('a successful settle clears staleness — no second refetch, for a mark set before the load starts', async () => {
         let counter = 0;
         const factory = vi.fn(() => delay(10).then(() => ++counter));
         const lazy = new LazyPromise(factory);
@@ -70,7 +70,7 @@ describe('LazyPromise.expireTracker', () => {
         await p1;
         expect(factory).toHaveBeenCalledTimes(1);
 
-        lazy.expireTracker.expire();
+        lazy.expireTracker.expire(); // marked stale before any load is in flight
         const p2 = lazy.promise;
         await vi.advanceTimersByTimeAsync(10);
         await p2;
@@ -226,21 +226,71 @@ describe('LazyPromise.expireTracker', () => {
         expect(lazy.value).toBe(2);
     });
 
-    test('expire() mid-flight is absorbed by the in-flight load\'s settle', async () => {
+    test('expire() mid-flight survives the in-flight load\'s settle', async () => {
         const factory = vi.fn(() => delay(10).then(() => 1));
         const lazy = new LazyPromise(factory);
 
         const p1 = lazy.promise;
         lazy.expireTracker.expire();
 
         await vi.advanceTimersByTimeAsync(10);
-        await p1;
+        await expect(p1).resolves.toBe(1);
 
-        expect(lazy.expireTracker.isExpired).toBeFalse();
+        expect(lazy.expireTracker.isForceExpired).toBeTrue();
+        expect(lazy.expireTracker.isExpired).toBeTrue();
         expect(factory).toHaveBeenCalledTimes(1);
         expect(lazy.pendingState).toBeNull();
     });
 
+    test('the mutation race: expire() mid-load survives the settle, and the next read re-invokes the factory', async () => {
+        let counter = 0;
+        const factory = vi.fn(() => delay(10).then(() => ++counter));
+        const lazy = new LazyPromise(factory);
+
+        const p1 = lazy.promise;
+        lazy.expireTracker.expire();
+
+        await vi.advanceTimersByTimeAsync(10);
+        await expect(p1).resolves.toBe(1);
+        expect(lazy.currentValue).toBe(1);
+        expect(factory).toHaveBeenCalledTimes(1);
+
+        const p2 = lazy.promise; // the mark survived the settle — this read re-invokes the factory
+        await vi.advanceTimersByTimeAsync(10);
+        await expect(p2).resolves.toBe(2);
+        expect(factory).toHaveBeenCalledTimes(2);
+    });
+
+    test('a load slower than its own lifetime settles as fresh, not stale', async () => {
+        const factory = vi.fn(() => delay(20).then(() => 1));
+        const lazy = new LazyPromise(factory).withExpire(10);
+
+        const p1 = lazy.promise;
+        await vi.advanceTimersByTimeAsync(20);
+        await expect(p1).resolves.toBe(1);
+
+        expect(lazy.expireTracker.isForceExpired).toBeFalse();
+        expect(lazy.expireTracker.isExpired).toBeFalse();
+        expect(lazy.currentValue).toBe(1);
+
+        // the settle restarted the tracker — a read right after resolve must not trigger a second load
+        void lazy.value;
+        expect(factory).toHaveBeenCalledTimes(1);
+    });
+
+    test('setInstance() after expireTracker.expire() clears the mark; a fetch settle does not', async () => {
+        const lazy = new LazyPromise(() => Promise.resolve(1));
+
+        lazy.expireTracker.expire();
+        expect(lazy.expireTracker.isForceExpired).toBeTrue();
+
+        lazy.setInstance(2);
+
+        expect(lazy.expireTracker.isForceExpired).toBeFalse();
+        expect(lazy.expireTracker.isExpired).toBeFalse();
+        expect(lazy.currentValue).toBe(2);
+    });
+
     test('a joined .value read mid-flight does not restart a shared tracker', async () => {
         const tracker = new ExpireTracker(1000);
         const lazy = new LazyPromise(() => delay(10).then(() => 1)).withExpire(tracker);

packages/common/src/lazy/__tests__/promise.state.test.ts

@@ -21,7 +21,7 @@ describe('LazyPromise', () => {
     // --- Deferred state write races under withAsyncStateChange ---
 
     test('revalidation resolving in the same microtask does not stick in "revalidating"', async () => {
-        const expiredTracker = { isExpired: true, restart: vi.fn(), expire: vi.fn() };
+        const expiredTracker = { isExpired: true, isForceExpired: false, restart: vi.fn(), expire: vi.fn() };
         let calls = 0;
         const lazy = new LazyPromise(() => {
             calls++;
@@ -43,7 +43,7 @@ describe('LazyPromise', () => {
     });
 
     test('refresh() followed by a same-tick .value read does not downgrade "refreshing"', async () => {
-        const expiredTracker = { isExpired: true, restart: vi.fn(), expire: vi.fn() };
+        const expiredTracker = { isExpired: true, isForceExpired: false, restart: vi.fn(), expire: vi.fn() };
         let resolveRefresh!: (value: string) => void;
         let calls = 0;
         const lazy = new LazyPromise(() => {

packages/common/src/lazy/lazy.ts

@@ -5,7 +5,8 @@ import type { ILazy } from './types.js';
 
 /**
  * Synchronous lazy-loading container that initializes a value on first access.
- * The value is cached until reset or expired. Supports custom disposal and cache expiration.
+ * The value is cached until reset or expired.
+ * Supports custom disposal and cache expiration.
  */
 export class Lazy<T> implements ILazy<T>, IDisposable, IResettableModel {
 
@@ -26,7 +27,10 @@ export class Lazy<T> implements ILazy<T>, IDisposable, IResettableModel {
     public get currentValue() { return this._instance; }
     public get error(): unknown { return this._error; }
 
-    /** The expiration tracker; once it expires, the next access resets and re-creates the value. Defaults to a never-expiring owned tracker. */
+    /**
+     * The expiration tracker; once it expires, the next access resets and re-creates the value.
+     * Defaults to a never-expiring owned tracker.
+     */
     public get expireTracker(): IExpireTracker {
         return this._expireTracker;
     }
@@ -65,7 +69,7 @@ export class Lazy<T> implements ILazy<T>, IDisposable, IResettableModel {
         return this;
     }
 
-    /** Eagerly loads the value without accessing it. Useful for preloading. */
+    /** Eagerly loads the value without accessing it. */
     public prewarm() {
         this.ensureInstance();
         return this;

packages/common/src/lazy/loadingState.ts

@@ -1,17 +1,19 @@
-import { DEFAULT_LOADING_STATE, type ILazyPromise, type LoadingStateStrategy, type PendingLoadState } from './types.js';
+import { DEFAULT_LOADING_STATE, type ILazyPromise, type LoadingStates, type LoadingStateStrategy, type PendingLoadState } from './types.js';
 import { LazyPromiseView } from './view.js';
 
 /**
  * Single loading-state policy module.
  *
- * A {@link PendingLoadState} classifies an in-flight load. A {@link LoadingStateStrategy} maps a
- * pending kind to the reported `isLoading`. A kind unnamed in the strategy falls through to a
- * caller-chosen fallback: an instance's own strategy falls back to the built-in defaults
- * ({@link DEFAULT_LOADING_STATE}), a view falls back to its source's report.
+ * A {@link PendingLoadState} classifies an in-flight load.
+ * A {@link LoadingStateStrategy} maps a pending kind to the reported `isLoading`.
+ *
+ * A kind unnamed in the strategy falls through to a caller-chosen fallback:
+ * - an instance's own strategy falls back to the built-in defaults ({@link DEFAULT_LOADING_STATE})
+ * - a view falls back to its source's report
  */
 
 /** Resolves `strategy[pending]`, falling back to `fallback()` when that entry is `undefined`. */
-export function resolveIsLoading<F extends boolean | null>(
+export function resolveIsLoading<F extends LoadingStates>(
     pending: PendingLoadState,
     strategy: LoadingStateStrategy | undefined,
     fallback: () => F,
@@ -26,9 +28,11 @@ export function deriveIsLoading(pending: PendingLoadState, strategy?: LoadingSta
 }
 
 /**
- * The pending kind for an explicit `refresh()` call: `'loading'`/`'refreshing'` already in flight keep
- * their classification — a passive `'revalidating'` is escalated to `'refreshing'`, since an explicit
- * refresh() is a stronger signal. Otherwise derived from whether a value already exists.
+ * The pending kind for an explicit `refresh()` call.
+ *
+ * - `'loading'`/`'refreshing'` already in flight keep their classification
+ * - a passive `'revalidating'` is escalated to `'refreshing'`, since an explicit `refresh()` is a stronger signal
+ * - otherwise derived from whether a value already exists
  */
 export function refreshPendingKind(current: PendingLoadState | null, hasValue: boolean): PendingLoadState {
     if (current === 'loading' || current === 'refreshing') {

packages/common/src/lazy/mapped.ts

@@ -2,14 +2,20 @@ import { Getter } from '../types/getter.js';
 import type { ILazyPromise, IResolvedLazyPromise } from './types.js';
 
 /**
- * Derives an always-valued {@link ILazyPromise} from a source by running a total mapper on every read.
- * `value` and `currentValue` are the override points for a subclass that wants to memoize the mapped result.
+ * Derives an always-valued {@link ILazyPromise} from a source, running a mapper that also handles `undefined` on every read.
+ *
+ * `value` and `currentValue` are the override points for memoizing the mapped result.
+ * `promise` and `refresh()` resolve through `this.value`, so an override reaches them too.
  */
 export class MappedLazyPromiseView<TSource, T, TSourceInitial extends TSource | undefined = undefined>
     implements ILazyPromise<T, T> {
 
     private readonly _sourceGetter: () => ILazyPromise<TSource, TSourceInitial>;
 
+    /**
+     * @param source Evaluated on every access, including inside `promise` and `refresh()`'s
+     * continuations — swapping its target mid-refresh resolves against the new source.
+     */
     constructor(
         source: Getter<ILazyPromise<TSource, TSourceInitial>>,
         protected readonly _map: (value: NoInfer<TSource | TSourceInitial> | undefined) => T,

packages/common/src/lazy/promise.ts

@@ -10,6 +10,7 @@ import type {
     IResolvedLazyPromise,
     LazyFactory,
     LazyPromiseOptions,
+    LoadingStates,
     LoadingStateStrategy,
     PendingLoadState,
 } from './types.js';
@@ -76,8 +77,14 @@ export class LazyPromise<T, TInitial extends T | undefined = undefined> implemen
         this._loadingStrategy = storage.createValue<LoadingStateStrategy | undefined>(undefined);
     }
 
-    /** Current loading state: true = loading, false = loaded, null = not started. Pending states report per {@link withLoadingState}. */
-    public get isLoading(): boolean | null {
+    /**
+     * The current loading state; see {@link LoadingStates}.
+     * Does not trigger loading.
+     *
+     * A pending load's reported value comes from the configured strategy.
+     * See {@link withLoadingState} and {@link LoadingStateStrategy}.
+     */
+    public get isLoading(): LoadingStates {
         const pending = this._pending.value;
         if (pending !== null) {
             return deriveIsLoading(pending, this._loadingStrategy.value);
@@ -123,10 +130,12 @@ export class LazyPromise<T, TInitial extends T | undefined = undefined> implemen
 
     /**
      * The expiration tracker driving revalidation; defaults to a never-expiring owned tracker.
-     * Expiring while a load is already in flight is absorbed by that load's successful settle —
-     * a resolved outcome restarts the tracker regardless of what happened to it in the meantime.
-     * Invoking a load begins a fresh lifetime, so a failed load does not retry until the lifetime
-     * elapses again.
+     *
+     * Restarts the lifetime:
+     * - when a load starts, so a failed load waits one out before retrying
+     * - when a load resolves, unless {@link IExpireTracker.isForceExpired}
+     *
+     * An `expire()` made mid-load therefore survives that load, and the next read revalidates.
      */
     public get expireTracker(): IExpireTracker {
         return this._expireTracker;
@@ -194,18 +203,30 @@ export class LazyPromise<T, TInitial extends T | undefined = undefined> implemen
     }
 
     public setInstance(res: T) {
+        return this.setResolved(res, true);
+    }
+
+    /**
+     * Stores `res` as the resolved value.
+     *
+     * @param forceRestartExpiration Restarts the expiration lifetime even while
+     * {@link IExpireTracker.isForceExpired}; otherwise that mark survives this write.
+     */
+    private setResolved(res: T, forceRestartExpiration = false): T {
         const prepared = this._prepareValue(res);
 
         this._transaction(() => {
             this.settle('resolved');
-            this.clearError(); // clear error on successful set
+            this.clearError();
             this._instance.value = prepared;
 
             // refresh promise so it won't keep old callbacks, resolved with the freshest value
             this._promise = Promise.resolve(prepared);
             this._activeFactoryPromise = null;
 
-            this._expireTracker.restart();
+            if (forceRestartExpiration || !this._expireTracker.isForceExpired) {
+                this._expireTracker.restart();
+            }
         });
 
         return prepared;
@@ -318,7 +339,6 @@ export class LazyPromise<T, TInitial extends T | undefined = undefined> implemen
         }
 
         // Restarting at invocation paces retries: a failed attempt still starts a lifetime.
-        // A successful settle restarts again.
         this._expireTracker.restart();
 
         let factoryResult: Promise<T> | T;
@@ -337,7 +357,7 @@ export class LazyPromise<T, TInitial extends T | undefined = undefined> implemen
                 }
 
                 if (this._activeFactoryPromise === factoryPromise) {
-                    return this.setInstance(res);
+                    return this.setResolved(res);
                 }
 
                 // Stale promise - return the latest active promise instead