Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions packages/common-firebase/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zajno/common-firebase",
"version": "4.3.7",
"version": "4.3.8",
"description": "Zajno's re-usable Firebase utilities for JS/TS projects",
"private": false,
"type": "module",
Expand Down Expand Up @@ -47,8 +47,8 @@
},
"peerDependencies": {
"@google-cloud/pubsub": "^5.2.2",
"@zajno/common": "^2.8.5",
"@zajno/common-mobx": "^1.7.3",
"@zajno/common": "~2.8.11",
"@zajno/common-mobx": "~1.7.6",
"firebase": "^11.6 || ^12.8",
"firebase-admin": "^12.7 || ^13.6",
"firebase-functions": "^7.0.5",
Expand Down
4 changes: 2 additions & 2 deletions packages/common-mobx/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zajno/common-mobx",
"version": "1.7.5",
"version": "1.7.6",
"description": "Zajno's re-usable utilities for JS/TS projects using MobX",
"private": false,
"type": "module",
Expand Down Expand Up @@ -40,7 +40,7 @@
"mobx": "^6.13.7"
},
"peerDependencies": {
"@zajno/common": "~2.8.10",
"@zajno/common": "~2.8.11",
"mobx": "^6.13",
"tslib": "^2.8"
},
Expand Down
132 changes: 131 additions & 1 deletion packages/common-mobx/src/structures/__tests__/promiseCache.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Disposer } from '@zajno/common/functions/disposer';
import { PromiseCacheObservable } from '../promiseCache.js';
import { reaction, runInAction } from 'mobx';
import { reaction, runInAction, configure } from 'mobx';

describe('PromiseCache observable', () => {
beforeEach(() => {
Expand Down Expand Up @@ -444,4 +444,134 @@ describe('PromiseCache observable', () => {

disposer.dispose();
});

// ─── Sticky error: no infinite re-fetch loop ─────────────────────────

it('reaction on getCurrent does NOT cause infinite fetch loop on error', async () => {
// Enforce actions to catch unprotected observable mutations
configure({ enforceActions: 'observed' });

const fetcher = vi.fn(async () => {
throw new Error('always fails');
});

const cache = new PromiseCacheObservable<string, string>(fetcher);

// First: verify that get() after error doesn't re-fetch (non-reactive)
await cache.get('a');
expect(fetcher).toHaveBeenCalledTimes(1);
expect(cache.getLastError('a')).toBeInstanceOf(Error);

// Second get() should NOT re-fetch
await cache.get('a');
expect(fetcher).toHaveBeenCalledTimes(1);

// Now test with reaction
fetcher.mockClear();
cache.clear(); // reset state

const valueHandler = vi.fn();
const disposer = new Disposer();

disposer.add(
reaction(
() => cache.getCurrent('a', false), // read without triggering fetch
v => valueHandler(v),
{ fireImmediately: true },
),
);

// Manually trigger fetch
await cache.get('a');

expect(fetcher).toHaveBeenCalledTimes(1);
expect(cache.getLastError('a')).toBeInstanceOf(Error);

// Subsequent get() should NOT re-fetch
await cache.get('a');
expect(fetcher).toHaveBeenCalledTimes(1);

disposer.dispose();
configure({ enforceActions: 'never' });
});

it('reaction on getLazy().value does NOT cause infinite fetch loop on error', async () => {
const fetcher = vi.fn(async () => {
throw new Error('always fails');
});

const cache = new PromiseCacheObservable<string, string>(fetcher);
const lazy = cache.getLazy('a');

// First: verify non-reactive behavior
await lazy.promise;
expect(fetcher).toHaveBeenCalledTimes(1);
expect(lazy.error).toBeInstanceOf(Error);

// Accessing value should NOT re-trigger fetch
void lazy.value;
expect(fetcher).toHaveBeenCalledTimes(1);

// Now test with reaction on currentValue (read-only, no fetch trigger)
fetcher.mockClear();
cache.clear();

const valueHandler = vi.fn();
const disposer = new Disposer();

disposer.add(
reaction(
() => lazy.currentValue,
v => valueHandler(v),
{ fireImmediately: true },
),
);

// Manually trigger fetch
await cache.get('a');

expect(fetcher).toHaveBeenCalledTimes(1);
expect(lazy.error).toBeInstanceOf(Error);

// Subsequent get() should NOT re-fetch
await cache.get('a');
expect(fetcher).toHaveBeenCalledTimes(1);

disposer.dispose();
});

it('after sticky error, refresh() still works in observable context', async () => {
let callCount = 0;
const fetcher = vi.fn(async () => {
callCount++;
if (callCount === 1) throw new Error('first call fails');
return `success-${callCount}`;
});

const cache = new PromiseCacheObservable<string, string>(fetcher);

const valueHandler = vi.fn();
const disposer = new Disposer();

// Initial fetch fails
await cache.get('a');
expect(fetcher).toHaveBeenCalledTimes(1);
expect(cache.getLastError('a')).toBeInstanceOf(Error);

// Set up reaction
disposer.add(
reaction(
() => cache.getCurrent('a', false),
v => valueHandler(v),
),
);

// refresh() should work and trigger the reaction
const result = await cache.refresh('a');
expect(result).toBe('success-2');
expect(cache.getLastError('a')).toBeNull();
expect(valueHandler).toHaveBeenCalledWith('success-2');

disposer.dispose();
});
});
8 changes: 4 additions & 4 deletions packages/common-mobx/src/structures/promiseCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class PromiseCacheObservable<T, K = string, TInitial extends T | undefine
| 'storeResult'
| 'onFetchComplete'
| 'onFetchSuperseded'
| '_set'
| '_deleteKey'
| 'clear'
| 'sanitize'
>(this, {
Expand All @@ -37,7 +37,7 @@ export class PromiseCacheObservable<T, K = string, TInitial extends T | undefine
storeResult: action,
onFetchComplete: action,
onFetchSuperseded: action,
_set: action,
_deleteKey: action,
clear: action,
sanitize: action,
});
Expand All @@ -54,8 +54,8 @@ export class PromiseCacheObservable<T, K = string, TInitial extends T | undefine
return new NumberModel();
}

protected pure_createItemsCache(): IMapModel<string, T | undefined> {
return observable.map<string, T | undefined>(undefined, { deep: false });
protected pure_createItemsCache(): IMapModel<string, T> {
return observable.map<string, T>(undefined, { deep: false });
}

protected pure_createItemsStatus(): IMapModel<string, boolean> {
Expand Down
2 changes: 1 addition & 1 deletion packages/common/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zajno/common",
"version": "2.8.10",
"version": "2.8.11",
"description": "Zajno's re-usable utilities for JS/TS projects",
"private": false,
"type": "module",
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/lazy/__tests__/promise.refresh.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('LazyPromise refresh', () => {
await lazy.promise;
expect(lazy.error).toBeInstanceOf(Error);
expect(lazy.errorMessage).toBe('Sync initial error');
expect(lazy.hasValue).toBeTrue();
expect(lazy.hasValue).toBeFalse();
expect(lazy.value).toBeUndefined();
});

Expand Down
45 changes: 44 additions & 1 deletion packages/common/src/lazy/__tests__/promise.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ describe('LazyPromise', () => {
await l.promise;
expect(l.error).toBeInstanceOf(Error);
expect(l.errorMessage).toBe('async error message');
expect(l.hasValue).toBeTrue();
expect(l.hasValue).toBeFalse();
expect(l.value).toBeUndefined();
}

Expand Down Expand Up @@ -280,4 +280,47 @@ describe('LazyPromise', () => {
expect(lazy.hasValue).toBeFalse();
});

// ─── hasResolvedValue type narrowing ─────────────────────────────────

test('hasResolvedValue narrows type after successful load', async () => {
const lazy = new LazyPromise(async () => {
await delay(10);
return { name: 'Alice' };
});

expect(lazy.hasResolvedValue()).toBe(false);

const p = lazy.promise;
await vi.advanceTimersByTimeAsync(10);
await p;

expect(lazy.hasResolvedValue()).toBe(true);

if (lazy.hasResolvedValue()) {
// Type narrowing: value is { name: string }, not { name: string } | undefined
const name: string = lazy.value.name;
expect(name).toBe('Alice');

const current: { name: string } = lazy.currentValue;
expect(current.name).toBe('Alice');
}
});

test('hasResolvedValue returns false after error', async () => {
const lazy = new LazyPromise(async () => {
throw new Error('fail');
});

await lazy.promise;

expect(lazy.hasResolvedValue()).toBe(false);
expect(lazy.hasValue).toBe(false);
expect(lazy.error).toBeInstanceOf(Error);
});

test('hasResolvedValue returns false before load', () => {
const lazy = new LazyPromise(async () => 42);
expect(lazy.hasResolvedValue()).toBe(false);
});

});
11 changes: 9 additions & 2 deletions packages/common/src/lazy/promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { tryDispose, type IDisposable } from '../functions/disposer.js';
import { formatError } from '../functions/safe.js';
import type { IResettableModel } from '../models/types.js';
import type { IExpireTracker } from '../structures/expire.js';
import type { IControllableLazyPromise, ILazyPromiseExtension, LazyFactory } from './types.js';
import type { IControllableLazyPromise, ILazyPromiseExtension, IResolvedLazyPromise, LazyFactory } from './types.js';

/**
* Asynchronous lazy-loading container that initializes via a promise-based factory.
Expand Down Expand Up @@ -42,9 +42,16 @@ export class LazyPromise<T, TInitial extends T | undefined = undefined> implemen

/** Current loading state: true = loading, false = loaded, null = not started */
public get isLoading() { return this._state; }
public get hasValue() { return this._state === false; }

/** Returns true if a value of type `T` has been successfully loaded (no error). */
public get hasValue() { return this._state === false && this._error == null; }
public get error(): unknown { return this._error; }

/** @inheritdoc */
public hasResolvedValue(): this is LazyPromise<T, TInitial> & IResolvedLazyPromise<T, TInitial> {
return this._state === false && this._error == null;
}

/** @deprecated Use {@link error} instead. */
public get errorMessage(): string | null {
return this._error != null ? formatError(this._error) : null;
Expand Down
Loading
Loading