Skip to content

Commit

Permalink
change: Promise behavior to create a new state observable on the node…
Browse files Browse the repository at this point in the history
… that can be accessed from the proxy without needing to be on the raw object
  • Loading branch information
jmeistrich committed Oct 8, 2023
1 parent 1cccb4a commit 135911e
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 27 deletions.
30 changes: 25 additions & 5 deletions src/ObservableObject.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { createObservable } from './createObservable';
import { beginBatch, endBatch, notify } from './batching';
import {
checkActivate,
Expand Down Expand Up @@ -25,7 +26,13 @@ import {
isPrimitive,
isPromise,
} from './is';
import type { ChildNodeValue, NodeValue, PromiseInfo, TrackingType } from './observableInterfaces';
import type {
ChildNodeValue,
NodeValue,
ObservableObject,
ObservableState,
TrackingType,
} from './observableInterfaces';
import { onChange } from './onChange';
import { updateTracking } from './tracking';

Expand Down Expand Up @@ -310,12 +317,12 @@ function updateNodes(parent: NodeValue, obj: Record<any, any> | Array<any> | und
return retValue ?? false;
}

export function getProxy(node: NodeValue, p?: string) {
export function getProxy(node: NodeValue, p?: string): ObservableObject {
// Get the child node if p prop
if (p !== undefined) node = getChildNode(node, p);

// Create a proxy if not already cached and return it
return node.proxy || (node.proxy = new Proxy<NodeValue>(node, proxyHandler));
return (node.proxy || (node.proxy = new Proxy<NodeValue>(node, proxyHandler))) as ObservableObject<any>;
}

const proxyHandler: ProxyHandler<any> = {
Expand Down Expand Up @@ -445,6 +452,10 @@ const proxyHandler: ProxyHandler<any> = {
return fnOrComputed;
}

if (p === 'state' && node.state) {
return node.state;
}

// Return an observable proxy to the property
return getProxy(node, p);
},
Expand Down Expand Up @@ -719,13 +730,22 @@ function updateNodesAndNotify(
}

export function extractPromise(node: NodeValue, value: Promise<any>) {
(value as PromiseInfo).status = 'pending';
if (!node.state) {
node.state = createObservable<ObservableState>(
{
isLoaded: false,
},
false,
getProxy,
) as ObservableObject<ObservableState>;
}
value
.then((value) => {
set(node, value);
node.state!.isLoaded.set(true);
})
.catch((error) => {
set(node, { error, status: 'rejected' } as PromiseInfo);
node.state!.error.set(error);
});
}

Expand Down
19 changes: 10 additions & 9 deletions src/observableInterfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ type NonPrimitiveKeys<T> = Pick<T, { [K in keyof T]-?: T[K] extends Primitive ?
type Recurse<T, K extends keyof T, TRecurse> = T[K] extends ObservableReadable
? T[K]
: T[K] extends Promise<infer t>
? Observable<t & PromiseInfo>
? Observable<t & WithState>
: T[K] extends Function
? T[K]
: T[K] extends ObservableProxyTwoWay<infer t, infer t2>
Expand Down Expand Up @@ -292,13 +292,17 @@ export interface ObservablePersistRemoteFunctions<T = any, TState = {}> {
params: ObservablePersistRemoteSetParams<T>,
): Promise<void | { changes?: object | undefined; dateModified?: number }>;
}

export interface ObservablePersistState {
isLoadedLocal: boolean;
export interface ObservableState {
isLoaded: boolean;
error?: Error;
}
export interface WithState {
state?: ObservableState;
}
export interface ObservablePersistState extends ObservableState {
isLoadedLocal: boolean;
isEnabledLocal: boolean;
isEnabledRemote: boolean;
error?: Error;
dateModified?: number;
clearLocal: () => Promise<void>;
sync: () => Promise<void>;
Expand Down Expand Up @@ -440,6 +444,7 @@ interface BaseNodeValue {
parentOther?: NodeValue;
functions?: Map<string, Function | ObservableComputed<any>>;
lazy?: boolean;
state?: Observable<ObservableState>;
}

export interface RootNodeValue extends BaseNodeValue {
Expand Down Expand Up @@ -489,7 +494,3 @@ export type ObservableProxyTwoWay<T extends Record<string, any>, T2> = {
} & ObservableBaseFns<T> & {
[symbolGetNode]: NodeValue;
};
export type PromiseInfo = {
error?: any;
status?: 'pending' | 'rejected';
};
30 changes: 17 additions & 13 deletions tests/tests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2137,43 +2137,44 @@ describe('Promise values', () => {
const promise = Promise.resolve(10);
const obs = observable({ promise });
expect(obs.promise).resolves.toEqual(10);
expect(obs.promise.status.get()).toEqual('pending');
expect(obs.promise.state.isLoaded.get()).toEqual(false);
await promise;
expect(obs.promise.get()).toEqual(10);
expect(obs.promise.state.isLoaded.get()).toEqual(true);
});
test('Promise child works when set later', async () => {
const obs = observable({ promise: undefined as unknown as Promise<number> });
let resolver: (value: number) => void;
const promise = new Promise<number>((resolve) => (resolver = resolve));
expect(obs.promise.status.get()).toEqual(undefined);
obs.promise.set(promise);
expect(obs.promise.status.get()).toEqual('pending');
expect(obs.promise.state.isLoaded.get()).toEqual(false);
// @ts-expect-error Fake error
resolver(10);
await promise;
expect(obs.promise.get()).toEqual(10);
expect(obs.promise.state.isLoaded.get()).toEqual(true);
});
test('Promise child works when assigned later', async () => {
const obs = observable({ promise: undefined as unknown as Promise<number> });
let resolver: (value: number) => void;
const promise = new Promise<number>((resolve) => (resolver = resolve));
expect(obs.promise.status.get()).toEqual(undefined);
obs.assign({ promise });
expect(obs.promise.status.get()).toEqual('pending');
expect(obs.promise.state.isLoaded.get()).toEqual(false);
// @ts-expect-error Fake error
resolver(10);
await promise;
expect(obs.promise.get()).toEqual(10);
expect(obs.promise.state.isLoaded.get()).toEqual(true);
});
test('Promise object becomes value', async () => {
const promise = Promise.resolve({ child: 10 });
const obs = observable(promise);

expect(obs.get().status).toEqual('pending');
expect(obs.status.get()).toEqual('pending');
expect(obs.state.isLoaded.get()).toEqual(false);
await promise;
expect(obs.get()).toEqual({ child: 10 });
expect(obs.child.get()).toEqual(10);
expect(obs.state.isLoaded.get()).toEqual(true);
});
test('Promise primitive becomes value', async () => {
const promise = Promise.resolve(10);
Expand All @@ -2197,14 +2198,16 @@ describe('Promise values', () => {
});
test('Promise value in set is error if it rejects', async () => {
const promise = Promise.reject('test');
const obs = observable<{ promise: number | undefined }>({ promise: undefined });
const obs = observable<{
promise: Promise<number>;
}>({ promise: undefined as any });
obs.promise.set(promise);
try {
await promise;
} catch {
await promiseTimeout(0);
}
expect(obs.promise.get()).toEqual({ error: 'test', status: 'rejected' });
expect(obs.promise.state.error.get()).toEqual('test');
});
test('when callback works with promises', async () => {
let resolver: (value: number) => void;
Expand Down Expand Up @@ -2238,7 +2241,7 @@ describe('Promise values', () => {
await promise;
// Still pending because it was not activated
expect(obs.promise).resolves.toEqual(10);
expect(obs.promise.status.get()).toEqual('pending');
expect(obs.promise.state.isLoaded.get()).toEqual(false);

// This get activates it but it takes a frame for it to equal the value
expect(obs.promise.get()).not.toEqual(10);
Expand Down Expand Up @@ -2511,12 +2514,12 @@ describe('Observable with promise', () => {
resolver = resolve;
});
const obs = observable(promise);
expect(obs.get().status).toEqual('pending');
expect(obs.state.isLoaded.get()).toEqual(false);
if (resolver) {
resolver(10);
}
await promiseTimeout(0);
expect(obs.get().status).toEqual(undefined);
expect(obs.state.isLoaded.get()).toEqual(true);
expect(obs.get()).toEqual(10);
});
test('when with promise observable', async () => {
Expand All @@ -2526,7 +2529,7 @@ describe('Observable with promise', () => {
});
const obs = observable(promise);

expect(obs.get().status).toEqual('pending');
expect(obs.state.isLoaded.get()).toEqual(false);

const fn = jest.fn();
when(() => obs.get() === 10, fn);
Expand All @@ -2538,6 +2541,7 @@ describe('Observable with promise', () => {
await promiseTimeout(1000);

expect(fn).toHaveBeenCalled();
expect(obs.state.isLoaded.get()).toEqual(true);
});
test('recursive batches prevented', async () => {
let isInInnerBatch = false;
Expand Down

0 comments on commit 135911e

Please sign in to comment.