Skip to content

Commit

Permalink
refactor: Removed explicit 'dependencies' option from compute()
Browse files Browse the repository at this point in the history
  • Loading branch information
@mnasyrov
mnasyrov committed Mar 13, 2023
1 parent f9e14cc commit 4e3b2eb
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 142 deletions.
22 changes: 1 addition & 21 deletions packages/rx-effects/benchmarks/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const ITERATION_COUNT = 100;

const bench = new Bench();

bench.add('reactive-computed-bench (implicit)', () => {
bench.add('reactive-computed-bench', () => {
const entry = createStore(0);

const a = compute((get) => get(entry));
Expand All @@ -25,26 +25,6 @@ bench.add('reactive-computed-bench (implicit)', () => {
}
});

bench.add('reactive-computed-bench (explicit)', () => {
const entry = createStore(0);

const a = compute(() => entry.get(), [entry]);
const b = compute(() => a.get() + 1, [a]);
const c = compute(() => a.get() + 1, [a]);
const d = compute(() => b.get() + c.get(), [b, c]);
const e = compute(() => d.get() + 1, [d]);
const f = compute(() => d.get() + e.get(), [d, e]);
const g = compute(() => d.get() + e.get(), [d, e]);
const h = compute(() => f.get() + g.get(), [f, g]);

h.value$.subscribe();

for (let i = 0; i < ITERATION_COUNT; i++) {
entry.set(i);
entry.notify();
}
});

async function main() {
await bench.run();

Expand Down
104 changes: 27 additions & 77 deletions packages/rx-effects/src/compute.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { queryBehaviourSubject } from './queryUtils';
import { createStore } from './store';

describe('compute()', () => {
it('should work #1', async () => {
it('should calculate the benchmark', async () => {
const entry = createStore(0);

const a = compute((get) => get(entry));
Expand Down Expand Up @@ -54,36 +54,9 @@ describe('compute()', () => {
expect(changes).toEqual([10, 18, 26]);
});

it('should work #2', async () => {
const entry = createStore(0);

const a = compute(() => entry.get(), [entry]);
const b = compute(() => a.get() + 1, [a]);
const c = compute(() => a.get() + 1, [a]);
const d = compute(() => b.get() + c.get(), [b, c]);
const e = compute(() => d.get() + 1, [d]);
const f = compute(() => d.get() + e.get(), [d, e]);
const g = compute(() => d.get() + e.get(), [d, e]);
const h = compute(() => f.get() + g.get(), [f, g]);

expect(h.get()).toEqual(10);

const changes = await collectChanges(h.value$, async () => {
entry.set(1);
expect(h.get()).toEqual(18);

await 0;

entry.set(2);
expect(h.get()).toEqual(26);
});

expect(changes).toEqual([10, 18, 26]);
});

it('should have typings to return another type', () => {
const source = createStore<number>(1);
const query: Query<string> = compute(() => source.get() + '!', [source]);
const query: Query<string> = compute((get) => get(source) + '!');
expect(query.get()).toBe('1!');
});

Expand Down Expand Up @@ -133,9 +106,10 @@ describe('compute()', () => {
const s2 = createStore(0);

const a = compute((get) => get(s1) + 1);
const b = compute((get) => ({ a: get(a), b: get(s2) }), {
comparator: (a, b) => a.b === b.b,
});
const b = compute(
(get) => ({ a: get(a), b: get(s2) }),
(a, b) => a.b === b.b,
);

expect(b.get()).toEqual({ a: 1, b: 0 });

Expand Down Expand Up @@ -216,8 +190,8 @@ describe('compute()', () => {
const bs = new BehaviorSubject<number>(1);
const source = queryBehaviourSubject(bs);

const query1 = compute(() => source.get() + 1, [source]);
const query2 = compute(() => query1.get() * 2, [query1]);
const query1 = compute((get) => get(source) + 1);
const query2 = compute((get) => get(query1) * 2);

const subject1 = new Subject();
const subject2 = new Subject();
Expand Down Expand Up @@ -255,8 +229,8 @@ describe('compute()', () => {
const bs = new BehaviorSubject<number>(1);
const source = queryBehaviourSubject(bs);

const query1 = compute(() => source.get() + 1, [source]);
const query2 = compute(() => query1.get() * 2, [query1]);
const query1 = compute((get) => get(source) + 1);
const query2 = compute((get) => get(query1) * 2);

const subject1 = new Subject();
const subject2 = new Subject();
Expand Down Expand Up @@ -291,20 +265,17 @@ describe('compute()', () => {
});

it('should throw an error on subscription to an incorrect dependency', async () => {
const source = createStore(1);

const query1 = compute(
() => 1,
[source, undefined as unknown as Query<number>],
);
const query1 = compute((get) => get(undefined as any));

const subject = new Subject();
const changes = await collectChanges(subject.pipe(materialize()), () => {
query1.value$.subscribe(subject);
});
expect(changes).toEqual([
{
error: new TypeError('Incorrect dependency'),
error: new TypeError(
"Cannot read properties of undefined (reading 'get')",
),
hasValue: false,
kind: 'E',
value: undefined,
Expand Down Expand Up @@ -399,9 +370,10 @@ describe('compute()', () => {
it('should use a custom comparator', async () => {
const source = createStore({ key: 1, val: 'a' });

const query = compute((get) => get(source), {
comparator: (a, b) => a.key === b.key,
});
const query = compute(
(get) => get(source),
(a, b) => a.key === b.key,
);

const changes = await collectChanges(query.value$, () => {
source.set({ key: 1, val: 'a' });
Expand Down Expand Up @@ -430,28 +402,6 @@ describe('compute()', () => {
});
});

describe('createComputationNode()', () => {
it('should create a default node', () => {
const node = createComputationNode(() => 1);

expect(node.customDeps).toBe(undefined);
});

it('should create a node by options', () => {
const s1 = createStore(1);
const s2 = createStore(2);

const comparator = jest.fn();
const node = createComputationNode(() => 1, {
comparator,
dependencies: [s1, s2, s1],
});

expect(node.comparator).toBe(comparator);
expect(node.customDeps).toEqual([s1, s2, s1]);
});
});

describe('createComputationQuery()', () => {
it('should return ComputationQuery', async () => {
const node = createComputationNode(() => 1);
Expand Down Expand Up @@ -534,16 +484,16 @@ describe('addValueObserver()', () => {
it('should add an observer and push a current value into it and make the node be hot', async () => {
const source = createStore(1);

const query1 = compute(() => 1, [source]);
const query1 = compute((get) => get(source) + 1);

const query2 = compute(() => 1, [query1]);
const query2 = compute((get) => get(query1) + 1);

const subject = new Subject();
const changes = await collectChanges(subject, () => {
query2.value$.subscribe(subject);
});

expect(changes).toEqual([1]);
expect(changes).toEqual([3]);
});
});

Expand Down Expand Up @@ -576,9 +526,9 @@ describe('removeValueObserver()', () => {
it('should remove an observer, make a node be cold if treeObserverCount = 0 and update parents', async () => {
const source = createStore(1);

const query1 = compute(() => 1, [source]);
const query1 = compute((get) => get(source) + 1);

const query2 = compute(() => 1, [query1]);
const query2 = compute((get) => get(query1) + 1);

const subject1 = new Subject();
const subject2 = new Subject();
Expand All @@ -592,9 +542,9 @@ describe('onSourceError()', () => {
const bs = new BehaviorSubject(1);
const source = queryBehaviourSubject(bs);

const query1 = compute(() => source.get() + 1, [source]);
const query1 = compute((get) => get(source) + 1);

const query2 = compute(() => query1.get() * 2, [query1]);
const query2 = compute((get) => get(query1) * 2);

const subject1 = new Subject();
const subject2 = new Subject();
Expand Down Expand Up @@ -634,9 +584,9 @@ describe('onSourceComplete()', () => {
const bs = new BehaviorSubject(1);
const source = queryBehaviourSubject(bs);

const query1 = compute(() => source.get() + 1, [source]);
const query1 = compute((get) => get(source) + 1);

const query2 = compute(() => query1.get() * 2, [query1]);
const query2 = compute((get) => get(query1) * 2);

const subject1 = new Subject();
const subject2 = new Subject();
Expand Down
42 changes: 8 additions & 34 deletions packages/rx-effects/src/compute.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ export type ComputationResolver = {
*/
export type Computation<T> = (resolver: ComputationResolver) => T;

/**
* Options for "compute()" function
*/
export type ComputationOptions<T> = {
/** A custom comparator to differ complex values */
comparator?: Comparator<T>;

/** Explicitly dependencies for refreshing calculations */
dependencies?: Query<unknown>[];
};

/**
* Creates a computable query which calculates its values by provided "computation" function and dependencies.
*
Expand Down Expand Up @@ -63,22 +52,13 @@ export type ComputationOptions<T> = {
* expect(messageUppercase.get()).toBe('HELLO WORLD!');
* ```
*/
export const compute: {
<T>(computation: Computation<T>, dependencies?: Query<unknown>[]): Query<T>;
<T>(computation: Computation<T>, options?: ComputationOptions<T>): Query<T>;
} = <T>(
export function compute<T>(
computation: Computation<T>,
dependenciesOrOptions?: Query<unknown>[] | ComputationOptions<T>,
) => {
const options: ComputationOptions<T> | undefined = dependenciesOrOptions
? Array.isArray(dependenciesOrOptions)
? { dependencies: dependenciesOrOptions }
: dependenciesOrOptions
: undefined;

const node = createComputationNode(computation, options);
comparator?: Comparator<T>,
): Query<T> {
const node = createComputationNode(computation, comparator);
return createComputationQuery(node);
};
}

/// INTERNAL

Expand All @@ -91,7 +71,6 @@ export type Node<T> = {
hot: boolean;
version?: number;
valueRef?: ValueRef<T>;
customDeps?: ReadonlyArray<Query<unknown>>;
resolvedDeps?: ReadonlySet<Query<unknown>>;
subscriptions?: (() => void)[];
observers?: Observer<T>[];
Expand All @@ -101,13 +80,12 @@ export type Node<T> = {
/** @internal */
export function createComputationNode<T>(
computation: Computation<T>,
options?: ComputationOptions<T>,
comparator?: Comparator<T>,
): Node<T> {
return {
hot: false,
computation,
comparator: options?.comparator,
customDeps: options?.dependencies,
comparator,
};
}

Expand Down Expand Up @@ -215,7 +193,7 @@ function makeHotNode<T>(node: Node<T>, observer: Observer<T>) {
valueRef = node.valueRef = calculate(node.computation);
}
} else {
const visitedDeps: Set<Query<unknown>> = new Set(node.customDeps);
const visitedDeps: Set<Query<unknown>> = new Set();

DEPS_COLLECTOR = (query) => {
if (!(query as any)._computed) visitedDeps.add(query);
Expand All @@ -232,10 +210,6 @@ function makeHotNode<T>(node: Node<T>, observer: Observer<T>) {

if (node.resolvedDeps.size > 0) {
node.resolvedDeps.forEach((parentQuery) => {
if (!parentQuery) {
throw new TypeError('Incorrect dependency');
}

if (!node.depObserver) {
node.depObserver = {
next: () => onSourceChanged(node),
Expand Down
6 changes: 1 addition & 5 deletions packages/rx-effects/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,5 @@ export { OBJECT_COMPARATOR } from './utils';
export type { StoreDeclaration, DeclaredStoreFactory } from './declareStore';
export { declareStore } from './declareStore';

export type {
ComputationOptions,
Computation,
ComputationResolver,
} from './compute';
export type { Computation, ComputationResolver } from './compute';
export { compute } from './compute';
4 changes: 2 additions & 2 deletions packages/rx-effects/src/queryMappers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function mapQuery<T, R>(
query: Query<T>,
mapper: (value: T) => R,
): Query<R> {
return compute((get) => mapper(get(query)), [query]);
return compute((get) => mapper(get(query)));
}

/**
Expand All @@ -33,5 +33,5 @@ export function mergeQueries<Values extends unknown[], Result>(
return compute((get) => {
const values = queries.map((query) => get(query));
return merger(...(values as Values));
}, queries);
});
}
4 changes: 1 addition & 3 deletions packages/rx-effects/src/store.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,7 @@ export function createStore<State>(
},

query<T>(selector?: (state: State) => T): Query<T> | Query<State> {
return selector
? compute<T>((get) => selector(get(store)), [store])
: store;
return selector ? compute<T>((get) => selector(get(store))) : store;
},

set(nextState: State) {
Expand Down

0 comments on commit 4e3b2eb

Please sign in to comment.